netdev - Re: [PATCH] net: Fix struct sock bitfield annotation

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: <19f34abd0910081454v51455ee0p30ad6715b5ee31c0@mail.gmail.com>
Date:	Thu, 8 Oct 2009 23:54:02 +0200
From:	Vegard Nossum <vegard.nossum@...il.com>
To:	Eric Dumazet <eric.dumazet@...il.com>
Cc:	"David S. Miller" <davem@...emloft.net>,
	Linux Netdev List <netdev@...r.kernel.org>,
	Ingo Molnar <mingo@...e.hu>
Subject: Re: [PATCH] net: Fix struct sock bitfield annotation

2009/10/8 Eric Dumazet <eric.dumazet@...il.com>:
> Since commit a98b65a3 (net: annotate struct sock bitfield), we lost 8 bytes
> in struct sock on 64bits arches because of kmemcheck_bitfield_end(flags) misplacement.
>
>
> struct good {
>        int             begin_flags[0];
>        unsigned char   a : 8;
>        unsigned char   b;
>        unsigned short  c;
>        int             end_flags[0];
>        int             sk_rcvbuf;
>        void           *ptr;
> };
> struct bad {
>        int             begin_flags[0];
>        unsigned char   a : 8;
>        int             end_flags[0];
>        unsigned char   b;
>        unsigned short  c;
>        int             sk_rcvbuf;
>        void           *ptr;
> };
> sizeof(struct good) = 16, sizeof(struct bad) = 24
>
> Signed-off-by: Eric Dumazet <eric.dumazet@...il.com>
> ---
>  include/net/sock.h |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/net/sock.h b/include/net/sock.h
> index 1621935..ecfb831 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -229,9 +229,9 @@ struct sock {
>        unsigned char           sk_shutdown : 2,
>                                sk_no_check : 2,
>                                sk_userlocks : 4;
> -       kmemcheck_bitfield_end(flags);
>        unsigned char           sk_protocol;
>        unsigned short          sk_type;
> +       kmemcheck_bitfield_end(flags);
>        int                     sk_rcvbuf;
>        socket_lock_t           sk_lock;
>        /*
>

Hm, no, this looks wrong to me, because sk_protocol and sk_type
_aren't_ in fact part of the bitfield.

We don't want to affect the kernel _at all_ when CONFIG_KMEMCHECK=n,
so I guess we should make the kmemcheck_bitfield_{begin|end}() macros
empty instead for that case? (And for kmemcheck kernels, we don't
really care about the lost 8 bytes anyway.)


Vegard