Messages in this thread | | | Subject | Re: [PATCH v4 07/10] x86/xsaves: Fix PTRACE frames for XSAVES | From | Dave Hansen <> | Date | Fri, 29 Apr 2016 13:25:34 -0700 |
| |
On 03/04/2016 10:12 AM, Yu-cheng Yu wrote: > + for (i = 0; i < XFEATURE_MAX; i++) { > + /* > + * Copy only in-use xstates. > + */ > + if (((header.xfeatures >> i) & 1) && xfeature_enabled(i)) { > + void *src = get_xsave_addr_no_check(xsave, i);
How could a bit in header.xfeatures get set if it is not set in xfeature_enabled() aka xfeatures_mask aka XCR0?
... > +int copyin_to_xsaves(const void *kbuf, const void __user *ubuf, > + struct xregs_state *xsave) > +{ > + unsigned int offset, size; > + int i; > + u64 xfeatures; > + > + offset = offsetof(struct xregs_state, header); > + size = sizeof(xfeatures); > + > + if (kbuf) > + memcpy(&xfeatures, kbuf + offset, size); > + else if (__copy_from_user(&xfeatures, ubuf + offset, size)) > + return -EFAULT; > + > + /* > + * Reject if the user tries to set any supervisor xstates. > + */ > + if (xfeatures & XFEATURE_MASK_SUPERVISOR) > + return -EINVAL; > + > + for (i = 0; i < XFEATURE_MAX; i++) { > + u64 mask = ((u64)1 << i); > + > + if ((xfeatures & mask) && xfeature_enabled(i)) { > + void *dst = get_xsave_addr_no_check(xsave, i); > + > + offset = xstate_offsets[i]; > + size = xstate_sizes[i]; > + > + if (kbuf) > + memcpy(dst, kbuf + offset, size); > + else if (__copy_from_user(dst, ubuf + offset, size)) > + return -EFAULT; > + } > + }
If a caller tries to pass a non-enabled xfeature in, we appear to just silently drop it and return success. Is that really what we want to do or do we want to error out?
| |