Re: [Cluster-devel] [PATCH v2] fs/dlm: Fix kernel memory disclosure
Hello, I wanted to ping the list and see if this could get a review: > Clear the 'unused' field and the uninitialized padding in 'lksb' to > avoid leaking memory to userland in copy_result_to_user(). > > Signed-off-by: Vlad Tsyrklevich > --- > fs/dlm/user.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/fs/dlm/user.c b/fs/dlm/user.c > index 1ce908c..83ddd47 100644 > --- a/fs/dlm/user.c > +++ b/fs/dlm/user.c > @@ -122,6 +122,8 @@ static void compat_input(struct dlm_write_request *kb, > static void compat_output(struct dlm_lock_result *res, > struct dlm_lock_result32 *res32) > { > + memset(res32, 0, sizeof(*res32)); > + > res32->version[0] = res->version[0]; > res32->version[1] = res->version[1]; > res32->version[2] = res->version[2];
Re: [Cluster-devel] [PATCH] fs/dlm: Fix kernel memory disclosure
You're right, thanks for double checking that logic! I've just sent an updated patch to the list. On Thu, Jan 26, 2017 at 5:54 PM Steven Whitehouse wrote: > > Hi, > > > On 26/01/17 08:54, Vlad Tsyrklevich wrote: > > Hello, I wanted to ping the list and see if this could get a review. > > > > On Mon, Jan 9, 2017 at 8:27 PM, Vlad Tsyrklevich > > wrote: > >> Clear the 'unused' field to avoid leaking memory to userland in > >> copy_result_to_user(). > >> > >> Signed-off-by: Vlad Tsyrklevich > >> --- > >> fs/dlm/user.c | 2 ++ > >> 1 file changed, 2 insertions(+) > >> > >> diff --git a/fs/dlm/user.c b/fs/dlm/user.c > >> index 1ce908c..0570711 100644 > >> --- a/fs/dlm/user.c > >> +++ b/fs/dlm/user.c > >> @@ -138,6 +138,8 @@ static void compat_output(struct dlm_lock_result *res, > >> res32->lksb.sb_flags = res->lksb.sb_flags; > >> res32->lksb.sb_lkid = res->lksb.sb_lkid; > >> res32->lksb.sb_lvbptr = (__u32)(long)res->lksb.sb_lvbptr; > >> + > >> + memset(&res32->unused, 0, sizeof(res32->unused)); > >> } > >> #endif > >> > >> -- > >> 2.7.0 > >> > It looks like struct dlm_lksb32 has a hole in it, so it would be safer > just to zero the whole of the dlm_lock_result32 before it is written to, > rather than trying to find all the holes individually, even if slightly > slower (I'm not sure it would be noticeable in reality though) > > Steve. >
[Cluster-devel] [PATCH v2] fs/dlm: Fix kernel memory disclosure
Clear the 'unused' field and the uninitialized padding in 'lksb' to avoid leaking memory to userland in copy_result_to_user(). Signed-off-by: Vlad Tsyrklevich --- fs/dlm/user.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/fs/dlm/user.c b/fs/dlm/user.c index 1ce908c..83ddd47 100644 --- a/fs/dlm/user.c +++ b/fs/dlm/user.c @@ -122,6 +122,8 @@ static void compat_input(struct dlm_write_request *kb, static void compat_output(struct dlm_lock_result *res, struct dlm_lock_result32 *res32) { + memset(res32, 0, sizeof(*res32)); + res32->version[0] = res->version[0]; res32->version[1] = res->version[1]; res32->version[2] = res->version[2]; -- 2.7.0
Re: [Cluster-devel] [PATCH] fs/dlm: Fix kernel memory disclosure
Hello, I wanted to ping the list and see if this could get a review. On Mon, Jan 9, 2017 at 8:27 PM, Vlad Tsyrklevich wrote: > Clear the 'unused' field to avoid leaking memory to userland in > copy_result_to_user(). > > Signed-off-by: Vlad Tsyrklevich > --- > fs/dlm/user.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/fs/dlm/user.c b/fs/dlm/user.c > index 1ce908c..0570711 100644 > --- a/fs/dlm/user.c > +++ b/fs/dlm/user.c > @@ -138,6 +138,8 @@ static void compat_output(struct dlm_lock_result *res, > res32->lksb.sb_flags = res->lksb.sb_flags; > res32->lksb.sb_lkid = res->lksb.sb_lkid; > res32->lksb.sb_lvbptr = (__u32)(long)res->lksb.sb_lvbptr; > + > + memset(&res32->unused, 0, sizeof(res32->unused)); > } > #endif > > -- > 2.7.0 >
[Cluster-devel] [PATCH] fs/dlm: Fix kernel memory disclosure
Clear the 'unused' field to avoid leaking memory to userland in copy_result_to_user(). Signed-off-by: Vlad Tsyrklevich --- fs/dlm/user.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/fs/dlm/user.c b/fs/dlm/user.c index 1ce908c..0570711 100644 --- a/fs/dlm/user.c +++ b/fs/dlm/user.c @@ -138,6 +138,8 @@ static void compat_output(struct dlm_lock_result *res, res32->lksb.sb_flags = res->lksb.sb_flags; res32->lksb.sb_lkid = res->lksb.sb_lkid; res32->lksb.sb_lvbptr = (__u32)(long)res->lksb.sb_lvbptr; + + memset(&res32->unused, 0, sizeof(res32->unused)); } #endif -- 2.7.0