lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <18d709710608301052u1307139dpf6e3b2da6e7bfcbe@mail.gmail.com>
Date:	Wed, 30 Aug 2006 14:52:33 -0300
From:	"Julio Auto" <mindvortex@...il.com>
To:	"David Wagner" <daw-usenet@...erner.cs.berkeley.edu>,
	schwidefsky@...ibm.com
Cc:	linux-kernel@...r.kernel.org
Subject: Re: [S390] cio: kernel stack overflow.

On 8/30/06, David Wagner <daw@...berkeley.edu> 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 majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ