> Date: Tue, 05 Apr 2011 14:27:02 +0300 > From: Tiago Vignatti <[email protected]> > > On 04/05/2011 03:14 PM, ext Simon Thum wrote: > > On 04/04/2011 07:54 PM, Tiago Vignatti wrote: > >> It will fix two possible cases of use after free in RemoveDevice. > >> > >> Signed-off-by: Tiago Vignatti<[email protected]> > >> --- > >> dix/devices.c | 1 + > >> 1 files changed, 1 insertions(+), 0 deletions(-) > >> > >> diff --git a/dix/devices.c b/dix/devices.c > >> index 534931c..0288e15 100644 > >> --- a/dix/devices.c > >> +++ b/dix/devices.c > >> @@ -941,6 +941,7 @@ CloseDevice(DeviceIntPtr dev) > >> free(dev->config_info); /* Allocated in xf86ActivateDevice. */ > >> dev->config_info = NULL; > >> dixFreeObjectWithPrivates(dev, PRIVATE_DEVICE); > >> + dev = NULL; > >> } > >> > >> /** > > OK, but _how_ does it do what you say it does? I'm just seeing a dead > > store to a local. > > if you check RemoveDevice, you will see two loops (for) with a > conditional (tmp = dev). If one of these conditional takes true path it > will call CloseDevice(tmp), which in turn will free(tmp) through > dixFreeObjectWithPrivates(dev, PRIVATE_DEVICE). Next, when it returns, > the conditional to stop the loop in RemoveDevice will test the value of > tmp, which cannot be done because was previously freed. > > hope you understood my explanation...
Nope. Your explanation can't be right. A dead store is a dead store. Any decent compiler will optimize away the statement you add, since it modifies a function argument (which are local variables in C since they're passed by value) and that argument isn't used in this function after you assign NULL to it. _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
