Message ID | xn1spcuyx4.fsf@greed.delorie.com |
---|---|
State | New |
Headers | show |
On 07/19/2017 01:19 PM, DJ Delorie wrote: > > I've checked in this hopefully obvious patch... It's not clear, but did you test this on s390x and ppc?
On Jul 19 2017, DJ Delorie <dj@redhat.com> wrote: > - savedmemcount = (size_t) *(savedend - sizeof (size_t)); > + savedmemcount = *(size_t *) (savedend - sizeof (size_t)); I don't see where savedend is aligned. Andreas.
Andreas Schwab <schwab@linux-m68k.org> writes: >> - savedmemcount = (size_t) *(savedend - sizeof (size_t)); >> + savedmemcount = *(size_t *) (savedend - sizeof (size_t)); > > I don't see where savedend is aligned. It's always a multiple of size_t away from the array we align for in __copy_grp, since that's where the data comes from. Unless there's *another* place where we put together that weird undocumented layout of the data - in which case, we shouln't ;-)
On Jul 19 2017, DJ Delorie <dj@redhat.com> wrote: > Andreas Schwab <schwab@linux-m68k.org> writes: >>> - savedmemcount = (size_t) *(savedend - sizeof (size_t)); >>> + savedmemcount = *(size_t *) (savedend - sizeof (size_t)); >> >> I don't see where savedend is aligned. > > It's always a multiple of size_t away from the array we align for in > __copy_grp, since that's where the data comes from. Thanks, I see that now. Andreas.
On 07/19/2017 07:19 PM, DJ Delorie wrote: > > I've checked in this hopefully obvious patch... > > From f8cef4d07d9641e27629bd3ce2d13f5d702fb251 Mon Sep 17 00:00:00 2001 > From: DJ Delorie <dj@delorie.com> > Date: Wed, 19 Jul 2017 13:14:34 -0400 > Subject: Fix cast-after-dereference > > Original code was dereferencing a char*, then casting the value > to size_t. Should cast the pointer to size_t* then deference. > > diff --git a/ChangeLog b/ChangeLog > index d514f08..8618e26 100644 > --- a/ChangeLog > +++ b/ChangeLog > @@ -1,3 +1,7 @@ > +2017-07-19 DJ Delorie <dj@delorie.com> > + > + * grp/grp-merge.c (libc_hidden_def): Fix cast-after-dereference. > + > 2017-07-19 H.J. Lu <hongjiu.lu@intel.com> > > [BZ #21741] > diff --git a/grp/grp-merge.c b/grp/grp-merge.c > index 6590e5d..035e7a6 100644 > --- a/grp/grp-merge.c > +++ b/grp/grp-merge.c > @@ -137,7 +137,7 @@ __merge_grp (struct group *savedgrp, char *savedbuf, char *savedend, > > /* Get the count of group members from the last sizeof (size_t) bytes in the > mergegrp buffer. */ > - savedmemcount = (size_t) *(savedend - sizeof (size_t)); > + savedmemcount = *(size_t *) (savedend - sizeof (size_t)); > > /* Get the count of new members to add. */ > for (memcount = 0; mergegrp->gr_mem[memcount]; memcount++) > I've pulled this patch and rerun the test on the mentioned systems. The test is now passing. Thanks. Stefan
diff --git a/ChangeLog b/ChangeLog index d514f08..8618e26 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,7 @@ +2017-07-19 DJ Delorie <dj@delorie.com> + + * grp/grp-merge.c (libc_hidden_def): Fix cast-after-dereference. + 2017-07-19 H.J. Lu <hongjiu.lu@intel.com> [BZ #21741] diff --git a/grp/grp-merge.c b/grp/grp-merge.c index 6590e5d..035e7a6 100644 --- a/grp/grp-merge.c +++ b/grp/grp-merge.c @@ -137,7 +137,7 @@ __merge_grp (struct group *savedgrp, char *savedbuf, char *savedend, /* Get the count of group members from the last sizeof (size_t) bytes in the mergegrp buffer. */ - savedmemcount = (size_t) *(savedend - sizeof (size_t)); + savedmemcount = *(size_t *) (savedend - sizeof (size_t)); /* Get the count of new members to add. */ for (memcount = 0; mergegrp->gr_mem[memcount]; memcount++)
I've checked in this hopefully obvious patch... From f8cef4d07d9641e27629bd3ce2d13f5d702fb251 Mon Sep 17 00:00:00 2001 From: DJ Delorie <dj@delorie.com> Date: Wed, 19 Jul 2017 13:14:34 -0400 Subject: Fix cast-after-dereference Original code was dereferencing a char*, then casting the value to size_t. Should cast the pointer to size_t* then deference.