On 20 May 2016 at 15:43, Kangjie Lu <kangjielu@xxxxxxxxx> wrote:
>
>
> On Friday, May 20, 2016, Vegard Nossum <vegard.nossum@xxxxxxxxx> wrote:
>>
>> On 19 May 2016 at 11:08, Jiri Slaby <jslaby@xxxxxxx> wrote:
>> > From: Kangjie Lu <kangjielu@xxxxxxxxx>
>> >
>> > 3.12-stable review patch. If anyone has any objections, please let me
>> > know.
>> >
>> > ===============
>> >
>> > [ Upstream commit 5f8e44741f9f216e33736ea4ec65ca9ac03036e6 ]
>> >
>> > The stack object “map” has a total size of 32 bytes. Its last 4
>> > bytes are padding generated by compiler. These padding bytes are
>> > not initialized and sent out via “nla_put”.
>> >
>> > Signed-off-by: Kangjie Lu <kjlu@xxxxxxxxxx>
>> > Signed-off-by: David S. Miller <davem@xxxxxxxxxxxxx>
>> > Signed-off-by: Jiri Slaby <jslaby@xxxxxxx>
>> > ---
>> > net/core/rtnetlink.c | 18 ++++++++++--------
>> > 1 file changed, 10 insertions(+), 8 deletions(-)
>> >
>> > diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
>> > index fd3a16e45dd9..5093f42d7afc 100644
>> > --- a/net/core/rtnetlink.c
>> > +++ b/net/core/rtnetlink.c
>> > @@ -950,14 +950,16 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb,
>> > struct net_device *dev,
>> > goto nla_put_failure;
>> >
>> > if (1) {
>> > - struct rtnl_link_ifmap map = {
>> > - .mem_start = dev->mem_start,
>> > - .mem_end = dev->mem_end,
>> > - .base_addr = dev->base_addr,
>> > - .irq = dev->irq,
>> > - .dma = dev->dma,
>> > - .port = dev->if_port,
>> > - };
>> > + struct rtnl_link_ifmap map;
>> > +
>> > + memset(&map, 0, sizeof(map));
>> > + map.mem_start = dev->mem_start;
>> > + map.mem_end = dev->mem_end;
>> > + map.base_addr = dev->base_addr;
>> > + map.irq = dev->irq;
>> > + map.dma = dev->dma;
>> > + map.port = dev->if_port;
>> > +
>> > if (nla_put(skb, IFLA_MAP, sizeof(map), &map))
>> > goto nla_put_failure;
>> > }
>> > --
>> > 2.8.2
>> >
>>
>> Just out of curiosity, was this observed in practice? I could be
>> wrong, but I was under the impression that using designated
>> initializers would zero the rest of the struct, including padding.
>
>
> Yes or no. According to my experiences, it depends on how
> it is initialized:
> if there are no variables but all constants in the bracket,
> a global initializer will be generated, which will zero the remaining bytes
> including padding; otherwise, no global initializer
> will be used, hence the remaining bytes are not initialized.
> In this case, dev is not a constant, so no global initializer
> will be used to initialize the padding bytes
I did some experiements with gcc and my observations are:
1. it doesn't depend on whether the initializer is constant or variable, but...
2. whether or not padding gets initialized depends on *which fields*
you're initializing (I assume this has to do with what instructions it
ends up using, as it might be faster to do a 32-bit mov on x86 instead
of an 8-bit one if you're initializing an 8-bit field which is
followed by 24 bits of padding, for example).
>> This seems to back that up:
>>
>> http://stackoverflow.com/a/3374468/1697183
>>
>> If this is indeed a real info leak, then I would assume we have much
>> bigger problems around the kernel.
>
>
> Could be. We've found many such bugs.
That is pretty sad. Anyway, thanks for fixing them.
Vegard
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html