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
| ||
|
Date: Thu, 26 Feb 2015 12:48:14 +0200 From: Eyal Birger <eyal.birger@...il.com> To: Shmulik Ladkani <shmulik.ladkani@...il.com> Cc: David Miller <davem@...emloft.net>, Willem de Bruijn <willemb@...gle.com>, Eric Dumazet <edumazet@...gle.com>, Marcel Holtmann <marcel@...tmann.org>, "netdev@...r.kernel.org" <netdev@...r.kernel.org> Subject: Re: [PATCH net-next 7/7] net: move skb->dropcount to skb->cb[] On Thu, Feb 26, 2015 at 12:17 PM, Eyal Birger <eyal.birger@...il.com> wrote: > On Thu, Feb 26, 2015 at 10:49 AM, Shmulik Ladkani > <shmulik.ladkani@...il.com> wrote: >> On Thu, 26 Feb 2015 04:10:12 +0200 Eyal Birger <eyal.birger@...il.com> wrote: >>> diff --git a/include/net/sock.h b/include/net/sock.h >>> index d462f5e..8807586 100644 >>> --- a/include/net/sock.h >>> +++ b/include/net/sock.h >>> @@ -2078,12 +2078,26 @@ static inline int sock_intr_errno(long timeo) >>> return timeo == MAX_SCHEDULE_TIMEOUT ? -ERESTARTSYS : -EINTR; >>> } >>> >>> +struct sock_skb_cb { >>> + u32 dropcount; >>> +}; >>> + >>> +/* Store sock_skb_cb at the end of skb->cb[] so protocol families >>> + * using skb->cb[] would keep using it directly and utilize its >>> + * alignement guarantee. >>> + */ >>> +#define SOCK_SKB_CB_OFFSET ((FIELD_SIZEOF(struct sk_buff, cb) - \ >>> + sizeof(struct sock_skb_cb))) >>> + >>> +#define SOCK_SKB_CB(__skb) ((struct sock_skb_cb *)((__skb)->cb + \ >>> + SOCK_SKB_CB_OFFSET)) >>> + >>> #define sock_skb_cb_check_size(size) \ >>> - BUILD_BUG_ON((size) > FIELD_SIZEOF(struct sk_buff, cb)) >>> + BUILD_BUG_ON((size) > SOCK_SKB_CB_OFFSET) >>> >> >> You didn't take care of aligning the 'sock_skb_cb'. >> (Althogh in practive, dropcount is 4 and cb[] is 48 so you're golden). >> >> How about: >> >> struct sock_skb_cb { >> /* protocol families specifc CBs */ >> u8 reserved[__SOCK_SKB_CB_OFFSET]; >> struct __sock_skb_cb x; // name me better! >> }; >> >> Where '__SOCK_SKB_CB_OFFSET' and '__sock_skb_cb' are as follows: >> >> struct __sock_skb_cb { // name me better! >> u32 dropcount; >> }; >> >> #define __SOCK_SKB_CB_OFFSET_UNALIGNED \ >> (FIELD_SIZEOF(struct sk_buff, cb) - sizeof(struct __sock_skb_cb)) >> >> #define __SOCK_SKB_CB_OFFSET \ >> (__SOCK_SKB_CB_OFFSET_UNALIGNED - \ >> (__SOCK_SKB_CB_OFFSET_UNALIGNED % __alignof__(struct __sock_skb_cb))) >> >> This also takes care for alignement of '__sock_skb_cb x' within the CB. >> >> Then, >> >> #define sock_skb_cb_check_size(size) \ >> BUILD_BUG_ON((size) > FIELD_SIZEOF(struct sock_skb_cb, reserved)) >> > > Agreed. Will add to v2. After giving it some additional thought and research, I don't think this is necessary. The sizeof operator on struct sock_skb_cb would give a padded result which would take care of the structure alignment. This is under the assumption that skb->cb[] size is of a proper multiple (which I think is rather safe). Objections? Eyal. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists