Messages in this thread | | | Date | Tue, 16 Nov 2010 23:16:20 +0300 | From | Vasiliy Kulikov <> | Subject | Re: [PATCH] ipc: explicitly clear stack memory for shminfo |
| |
Hi Kees,
On Tue, Nov 16, 2010 at 11:58 -0800, Kees Cook wrote: > This fixes a kernel stack memory contents leak by explicitly clearing > the shminfo structure on the kernel stack before it is populated and > copied back to userspace. > > Signed-off-by: Kees Cook <kees.cook@canonical.com> > Acked-by: Pekka Enberg <penberg@kernel.org> > --- > ipc/shm.c | 1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/ipc/shm.c b/ipc/shm.c > index 7d3bb22..1d3d41f 100644 > --- a/ipc/shm.c > +++ b/ipc/shm.c > @@ -531,6 +531,7 @@ static inline unsigned long copy_shminfo_to_user(void __user *buf, struct shminf > { > struct shminfo out; > > + memset(&out, 0, sizeof(out)); > if(in->shmmax > INT_MAX) > out.shmmax = INT_MAX; > else > -- > 1.7.2.3
Can you please clarify what fields (padding bytes?) are uninitialized here? I see this struct shminfo definition:
/* Obsolete, used only for backwards compatibility */ struct shminfo { int shmmax; int shmmin; int shmmni; int shmseg; int shmall; };
And this filling:
if(in->shmmax > INT_MAX) out.shmmax = INT_MAX; else out.shmmax = (int)in->shmmax;
out.shmmin = in->shmmin; out.shmmni = in->shmmni; out.shmseg = in->shmseg; out.shmall = in->shmall;
return copy_to_user(buf, &out, sizeof(out));
As I see all five fields are set anyway, no padding bytes here, correct?
Thanks,
-- Vasiliy Kulikov http://www.openwall.com - bringing security into open computing environments
| |