netdev - Is it safe for a NIC driver to use all the 48 bytes of skb->cb?

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-next>] [day] [month] [year] [list]

Message-ID: <HK0P153MB0148311C48144413792A0FBEBF140@HK0P153MB0148.APCP153.PROD.OUTLOOK.COM>
Date:   Sat, 15 Feb 2020 05:23:55 +0000
From:   Dexuan Cui <decui@...rosoft.com>
To:     Haiyang Zhang <haiyangz@...rosoft.com>,
        Stephen Hemminger <sthemmin@...rosoft.com>,
        "David S. Miller" <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        KY Srinivasan <kys@...rosoft.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC:     "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>
Subject: Is it safe for a NIC driver to use all the 48 bytes of skb->cb?

Hi,
It looks all the layers of drivers among the network stack can use the 48-byte
skb->cb array. Is there any rule how they should coordinate with each other?

I noticed the last 16 bytes are used by struct skb_gso_cb:

include/linux/skbuff.h:
struct skb_gso_cb {
        union {
                int     mac_offset;
                int     data_offset;
        };
        int     encap_level;
        __wsum  csum;
        __u16   csum_start;
};
#define SKB_SGO_CB_OFFSET       32
#define SKB_GSO_CB(skb) ((struct skb_gso_cb *)((skb)->cb + SKB_SGO_CB_OFFSET))

Does this mean a low level NIC driver (e.g. hv_netvsc) should only use
the first 32 bytes? What if the upper layer network stack starts to take up
more space in the future? 

Now hv_netvsc assumes it can use all of the 48-bytes, though it uses only 
20 bytes, but just in case the struct hv_netvsc_packet grows to >32 bytes in the
future, should we change the BUILD_BUG_ON() in netvsc_start_xmit() to
 BUILD_BUG_ON(sizeof(struct hv_netvsc_packet) > SKB_SGO_CB_OFFSET);
?  

struct hv_netvsc_packet {
        /* Bookkeeping stuff */
        u8 cp_partial; /* partial copy into send buffer */

        u8 rmsg_size; /* RNDIS header and PPI size */
        u8 rmsg_pgcnt; /* page count of RNDIS header and PPI */
        u8 page_buf_cnt;

        u16 q_idx;
        u16 total_packets;

        u32 total_bytes;
        u32 send_buf_index;
        u32 total_data_buflen;
};

static int netvsc_start_xmit(struct sk_buff *skb, struct net_device *net)
{
...
        /*
         * Place the rndis header in the skb head room and
         * the skb->cb will be used for hv_netvsc_packet
         * structure.
         */
        ret = skb_cow_head(skb, RNDIS_AND_PPI_SIZE);
        if (ret)
                goto no_memory;

        /* Use the skb control buffer for building up the packet */
        BUILD_BUG_ON(sizeof(struct hv_netvsc_packet) >
                        FIELD_SIZEOF(struct sk_buff, cb));
        packet = (struct hv_netvsc_packet *)skb->cb;

Thanks,
-- Dexuan