From: Al Viro <viro@zeniv.linux.org.uk> To: Alexander Potapenko <glider@google.com> Cc: Kees Cook <keescook@chromium.org>, Andrew Morton <akpm@linux-foundation.org>, Alexey Dobriyan <adobriyan@gmail.com>, LKML <linux-kernel@vger.kernel.org>, sunhaoyl@outlook.com, x86@kernel.org Subject: Re: [PATCH] fs/binfmt_elf.c: allocate initialized memory in fill_thread_core_info() Date: Tue, 26 May 2020 23:38:17 +0100 [thread overview] Message-ID: <20200526223817.GA3819674@ZenIV.linux.org.uk> (raw) In-Reply-To: <20200524234535.GA23230@ZenIV.linux.org.uk> On Mon, May 25, 2020 at 12:45:35AM +0100, Al Viro wrote: > On Wed, May 13, 2020 at 04:33:49AM +0100, Al Viro wrote: > > > FWIW, what I'm going to do is > > * make all callers of copy_regset_to_user() pass 0 as pos > > (there are very few exceptions - one on arm64, three on sparc32 > > and five on sparc64; I hadn't dealt with arm64 one yet, but all > > cases on sparc are handled) > > [snip] > > Any of that would be easy to backport, though. Several questions > regaring XSAVE and friends: > > * do we ever run on XSAVE/XSAVES-capable hardware with XFEATURE_FP > turned off? > > * is it possible for x86 to have gaps between the state components > area as reported by CPUID 0x0d? IOW, can area for feature 2 > (XFEATURE_YMM) to start *not* at 0x200 and can area for N start > not right after the end of area for N-1 for some N > 2? > > I think I have an easy-to-backport solution, but I'm really confused > about XFEATURE_FP situation... Folks, could you test the following? copy_xstate_to_kernel(): don't leave parts of destination uninitialized copy the corresponding pieces of init_fpstate into the gaps instead. Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> --- diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c index 32b153d38748..6a54e83d5589 100644 --- a/arch/x86/kernel/fpu/xstate.c +++ b/arch/x86/kernel/fpu/xstate.c @@ -957,18 +957,31 @@ static inline bool xfeatures_mxcsr_quirk(u64 xfeatures) return true; } -/* - * This is similar to user_regset_copyout(), but will not add offset to - * the source data pointer or increment pos, count, kbuf, and ubuf. - */ -static inline void -__copy_xstate_to_kernel(void *kbuf, const void *data, - unsigned int offset, unsigned int size, unsigned int size_total) +static void fill_gap(unsigned to, void **kbuf, unsigned *pos, unsigned *count) { - if (offset < size_total) { - unsigned int copy = min(size, size_total - offset); + if (*pos < to) { + unsigned size = to - *pos; + + if (size > *count) + size = *count; + memcpy(*kbuf, (void *)&init_fpstate.xsave + *pos, size); + *kbuf += size; + *pos += size; + *count -= size; + } +} - memcpy(kbuf + offset, data, copy); +static void copy_part(unsigned offset, unsigned size, void *from, + void **kbuf, unsigned *pos, unsigned *count) +{ + fill_gap(offset, kbuf, pos, count); + if (size > *count) + size = *count; + if (size) { + memcpy(*kbuf, from, size); + *kbuf += size; + *pos += size; + *count -= size; } } @@ -981,8 +994,9 @@ __copy_xstate_to_kernel(void *kbuf, const void *data, */ int copy_xstate_to_kernel(void *kbuf, struct xregs_state *xsave, unsigned int offset_start, unsigned int size_total) { - unsigned int offset, size; struct xstate_header header; + const unsigned off_mxcsr = offsetof(struct fxregs_state, mxcsr); + unsigned count = size_total; int i; /* @@ -998,46 +1012,42 @@ int copy_xstate_to_kernel(void *kbuf, struct xregs_state *xsave, unsigned int of header.xfeatures = xsave->header.xfeatures; header.xfeatures &= ~XFEATURE_MASK_SUPERVISOR; + if (header.xfeatures & XFEATURE_MASK_FP) + copy_part(0, off_mxcsr, + &xsave->i387, &kbuf, &offset_start, &count); + if (header.xfeatures & (XFEATURE_MASK_SSE | XFEATURE_MASK_YMM)) + copy_part(off_mxcsr, MXCSR_AND_FLAGS_SIZE, + &xsave->i387.mxcsr, &kbuf, &offset_start, &count); + if (header.xfeatures & XFEATURE_MASK_FP) + copy_part(offsetof(struct fxregs_state, st_space), 128, + &xsave->i387.st_space, &kbuf, &offset_start, &count); + if (header.xfeatures & XFEATURE_MASK_SSE) + copy_part(xstate_offsets[XFEATURE_MASK_SSE], 256, + &xsave->i387.xmm_space, &kbuf, &offset_start, &count); + /* + * Fill xsave->i387.sw_reserved value for ptrace frame: + */ + copy_part(offsetof(struct fxregs_state, sw_reserved), 48, + xstate_fx_sw_bytes, &kbuf, &offset_start, &count); /* * Copy xregs_state->header: */ - offset = offsetof(struct xregs_state, header); - size = sizeof(header); - - __copy_xstate_to_kernel(kbuf, &header, offset, size, size_total); + copy_part(offsetof(struct xregs_state, header), sizeof(header), + &header, &kbuf, &offset_start, &count); - for (i = 0; i < XFEATURE_MAX; i++) { + for (i = FIRST_EXTENDED_XFEATURE; i < XFEATURE_MAX; i++) { /* * Copy only in-use xstates: */ if ((header.xfeatures >> i) & 1) { void *src = __raw_xsave_addr(xsave, i); - offset = xstate_offsets[i]; - size = xstate_sizes[i]; - - /* The next component has to fit fully into the output buffer: */ - if (offset + size > size_total) - break; - - __copy_xstate_to_kernel(kbuf, src, offset, size, size_total); + copy_part(xstate_offsets[i], xstate_sizes[i], + src, &kbuf, &offset_start, &count); } } - - if (xfeatures_mxcsr_quirk(header.xfeatures)) { - offset = offsetof(struct fxregs_state, mxcsr); - size = MXCSR_AND_FLAGS_SIZE; - __copy_xstate_to_kernel(kbuf, &xsave->i387.mxcsr, offset, size, size_total); - } - - /* - * Fill xsave->i387.sw_reserved value for ptrace frame: - */ - offset = offsetof(struct fxregs_state, sw_reserved); - size = sizeof(xstate_fx_sw_bytes); - - __copy_xstate_to_kernel(kbuf, xstate_fx_sw_bytes, offset, size, size_total); + fill_gap(size_total, &kbuf, &offset_start, &count); return 0; }
next prev parent reply other threads:[~2020-05-26 22:38 UTC|newest] Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-04-19 10:08 glider 2020-04-19 10:10 ` Alexander Potapenko 2020-04-20 22:33 ` Andrew Morton 2020-04-20 22:41 ` Kees Cook 2020-04-21 3:42 ` Al Viro 2020-04-21 8:14 ` Alexander Potapenko 2020-05-12 1:09 ` Al Viro 2020-05-12 3:44 ` Al Viro 2020-05-12 8:20 ` Alexander Potapenko 2020-05-13 3:33 ` Al Viro 2020-05-24 23:45 ` Al Viro 2020-05-26 22:38 ` Al Viro [this message] 2020-05-27 12:08 ` Alexander Potapenko 2020-05-27 19:04 ` Borislav Petkov 2020-05-27 19:53 ` Al Viro 2020-05-27 20:09 ` Borislav Petkov 2020-04-21 12:54 ` Alexander Potapenko 2020-04-21 15:09 ` Jann Horn 2020-04-21 16:04 ` Yu-cheng Yu 2020-04-21 16:16 ` Jann Horn 2020-04-21 16:26 ` Yu-cheng Yu 2020-04-21 20:20 ` Kees Cook 2020-04-21 8:06 ` Alexander Potapenko 2020-05-27 21:55 ` Kees Cook 2020-04-21 8:00 ` Alexander Potapenko
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20200526223817.GA3819674@ZenIV.linux.org.uk \ --to=viro@zeniv.linux.org.uk \ --cc=adobriyan@gmail.com \ --cc=akpm@linux-foundation.org \ --cc=glider@google.com \ --cc=keescook@chromium.org \ --cc=linux-kernel@vger.kernel.org \ --cc=sunhaoyl@outlook.com \ --cc=x86@kernel.org \ --subject='Re: [PATCH] fs/binfmt_elf.c: allocate initialized memory in fill_thread_core_info()' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).