On 8/30/06, David Wagner <[email protected]> wrote:
Have you checked that in all cases all fields of the struct have
been overwritten? For instance, look at this:
Martin Schwidefsky wrote:
>- chp->dev = (struct device) {
>- .parent = &css[0]->device,
>- .release = chp_release,
>- };
>+ chp->dev.parent = &css[0]->device;
>+ chp->dev.release = chp_release;
Doesn't this leave chp->dev.bus still holding whatever old value it
had laying around before?
You're correct. While this eliminates the possibility of getting
random values from the stack, it still leaves space for letting
previous unwanted values unchanged.
Unless, of course, the structure in question is kcalloc()'d (which is
not the case of gdev in the beginning of the patch - I haven't had the
time to check the other cases). But we surely can't rely on that.
Unless I'm missing something, it looks to
me like this diff causes a change in the semantics of the code.
I can't see the semantic change.
Perhaps it would be better to memset() the entire struct (chp->dev, in
this case) to zero, before assigning to individual fields, so there is
no possibility of old remnant data still being left laying around?
Yes. Since the code can't be depend on the caller passing a zeroed
structure, it's definitely more safe to memset to 0 (or use kcalloc(),
instead of kmalloc(), when the allocation is the routine's own
responsability).
Changing subjects a little bit: where's the stack overflow in the
code? Either I'm too stupid to find it myself by looking at the patch
or the e-mail is mistitled.
Cheers,
Julio Auto
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
