[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 18 Dec 2014 22:35:37 +1300
From: Amos Jeffries <squid3@...enet.co.nz>
To: oss-security@...ts.openwall.com
Subject: Re: How GNU/Linux distros deal with offset2lib attack?
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 18/12/2014 9:24 p.m., Lionel Debroux wrote:
>
> In addition to what I wrote earlier: PaX contains several hundreds
> of lines of hunks dealing with local variables needlessly made
> static: ============================== ---
> linux-3.17.6/drivers/mfd/max8925-i2c.c +++
> linux-3.17.6-pax/drivers/mfd/max8925-i2c.c @@ -152,7 +152,7 @@
> static int max8925_probe(struct i2c_clie const struct i2c_device_id
> *id) { struct max8925_platform_data *pdata =
> dev_get_platdata(&client->dev); - static struct max8925_chip
> *chip; + struct max8925_chip *chip; struct device_node *node =
> client->dev.of_node;
>
> if (node && !pdata) {
>
> (the first reference to the "chip" variable in that function is an
> unconditional devm_kzalloc)
NP: I have not looked at either version of code outside the thread
here. Just responding to your statement of needless...
The above sounds to me like the author wanted the alloc to only happen
once, lazily on first use and remain allocated until the kerel or
module was released. Or perhapse they wanted data in it to persist
between calls.
Neither of those cases is necessarily needless. But its utility does
depend on how often the function is called. Saving a handful of rare
event allocations per kernel lifetime is almost needless (unless they
happen to all occur in a batch at some critical point). Saving
thousands per second is very much useful.
In the former case security is best served by removing the static, in
the latter it is served by ensuring the struct content is fully
cleaned or revalidated before use in each call.
- From my long experience lurking on some of the mainline dev lists ...
in order to get such "trivial" patches merged you will have to justify
that you at least considered and investigated which cases like the
above was the cause of the codes current form. And what the effect of
the proposed change would be in both the security and performance arenas.
People using PaX code are trusting that they have done the analysis,
but that very code not being in mainline means there is possibly no
hard proof of that. PaX may have decided that a huge performance
penalty for some odd-ball drivers was worth some minor security gain
for everybody.
> ============================== or local structs which are not meant
> to be modified and should therefore probably be made static /
> static const (mainline doesn't use the GCC plugin for
> constification): ============================== ---
> linux-3.17.6/arch/arm/mach-omap2/wd_timer.c +++
> linux-3.17.6-pax/arch/arm/mach-omap2/wd_timer.c @@ -110,7 +110,9 @@
> static int __init omap_init_wdt(void) struct omap_hwmod *oh; char
> *oh_name = "wd_timer2"; char *dev_name = "omap_wdt"; - struct
> omap_wd_timer_platform_data pdata; + static struct
> omap_wd_timer_platform_data pdata = { + .read_reset_sources
> = prm_read_reset_sources + };
>
> if (!cpu_class_is_omap2() || of_have_populated_dt()) return 0; @@
> -121,8 +123,6 @@ static int __init omap_init_wdt(void) return
> -EINVAL; }
>
> - pdata.read_reset_sources = prm_read_reset_sources; - pdev =
> omap_device_build(dev_name, id, oh, &pdata, sizeof(struct
> omap_wd_timer_platform_data)); WARN(IS_ERR(pdev), "Can't build
> omap_device for %s:%s.\n", ==============================
>
Now *that* does just appear to be a gratuitous cleanup / performance
booster. Not security related.
If there is a security angle to it I have an interest in learning what
that is exactly. Implicit NULL'ing by the compiler?
AYJ
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (MingW32)
iQEcBAEBAgAGBQJUkp/pAAoJELJo5wb/XPRjn6UH+gOROtCMf3KEmeI2BOwr5d+O
aY9G49rs3BvxPFo8DkEjU4ON3QdlO+inhAFDYW/dYbjK7eJsl+OvMaDGZxZbFJMw
BBAXf5fuS4gMLeLyTke8GEp9OfUjxWN8FAzlIFf9ueLLSZfevvJ9aHWEgu732TAf
vu926kCnakoI4jiytAV+Tig8XRkljHTlEKhKIFknfIssLvAc5ffrQW38MmtZpj8c
29dt1GMQ1AIVhi50FjTRMeTubjGzNNQvUsNub0G5D8Q/6yOKUYoQUxqIVUidExN6
IlueeBS1J+ElNNtFcCV1Y9rGllL60N5kdHi9/SFS6PhE4vMkKaSC2bY7/rB8gSI=
=FRUD
-----END PGP SIGNATURE-----
Powered by blists - more mailing lists
Please check out the
Open Source Software Security Wiki, which is counterpart to this
mailing list.
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
