Skip to content

Incorrect bindings generation with sk_buff C structure from Linux kernel #3089

Closed
@MikeRomaniuk

Description

@MikeRomaniuk

I've used a bindgen to generate bindings to the sk_buff structure and I've got 2 identical variables in different __BindgenUnionField.
Original C structure:

struct sk_buff {
	union {
		struct {
			/* These two members must be first to match sk_buff_head. */
			struct sk_buff		*next;
			struct sk_buff		*prev;

			union {
				struct net_device	*dev;
				/* Some protocols might use this space to store information,
				 * while device pointer would be NULL.
				 * UDP receive path is one user.
				 */
				unsigned long		dev_scratch;
			};
		};
		struct rb_node		rbnode; /* used in netem, ip4 defrag, and tcp stack */
		struct list_head	list;
		struct llist_node	ll_node;
	};

	struct sock		*sk;

	union {
		ktime_t		tstamp;
		u64		skb_mstamp_ns; /* earliest departure time */
	};
	/*
	 * This is the control buffer. It is free to use for every
	 * layer. Please put your private variables there. If you
	 * want to keep them across layers you have to do a skb_clone()
	 * first. This is owned by whoever has the skb queued ATM.
	 */
	char			cb[48] __aligned(8);

	union {
		struct {
			unsigned long	_skb_refdst;
			void		(*destructor)(struct sk_buff *skb);
		};
		struct list_head	tcp_tsorted_anchor;
#ifdef CONFIG_NET_SOCK_MSG
		unsigned long		_sk_redir;
#endif
	};

#if defined(CONFIG_NF_CONNTRACK) || defined(CONFIG_NF_CONNTRACK_MODULE)
	unsigned long		 _nfct;
#endif
	unsigned int		len,
				data_len;
	__u16			mac_len,
				hdr_len;

	/* Following fields are _not_ copied in __copy_skb_header()
	 * Note that queue_mapping is here mostly to fill a hole.
	 */
	__u16			queue_mapping;

/* if you move cloned around you also must adapt those constants */
#ifdef __BIG_ENDIAN_BITFIELD
#define CLONED_MASK	(1 << 7)
#else
#define CLONED_MASK	1
#endif
#define CLONED_OFFSET		offsetof(struct sk_buff, __cloned_offset)

	/* private: */
	__u8			__cloned_offset[0];
	/* public: */
	__u8			cloned:1,
				nohdr:1,
				fclone:2,
				peeked:1,
				head_frag:1,
				pfmemalloc:1,
				pp_recycle:1; /* page_pool recycle indicator */
#ifdef CONFIG_SKB_EXTENSIONS
	__u8			active_extensions;
#endif

	/* Fields enclosed in headers group are copied
	 * using a single memcpy() in __copy_skb_header()
	 */
	struct_group(headers,

	/* private: */
	__u8			__pkt_type_offset[0];
	/* public: */
	__u8			pkt_type:3; /* see PKT_TYPE_MAX */
	__u8			ignore_df:1;
	__u8			dst_pending_confirm:1;
	__u8			ip_summed:2;
	__u8			ooo_okay:1;

	/* private: */
	__u8			__mono_tc_offset[0];
	/* public: */
	__u8			mono_delivery_time:1;	/* See SKB_MONO_DELIVERY_TIME_MASK */
#ifdef CONFIG_NET_XGRESS
	__u8			tc_at_ingress:1;	/* See TC_AT_INGRESS_MASK */
	__u8			tc_skip_classify:1;
#endif
	__u8			remcsum_offload:1;
	__u8			csum_complete_sw:1;
	__u8			csum_level:2;
	__u8			inner_protocol_type:1;

	__u8			l4_hash:1;
	__u8			sw_hash:1;
#ifdef CONFIG_WIRELESS
	__u8			wifi_acked_valid:1;
	__u8			wifi_acked:1;
#endif
	__u8			no_fcs:1;
	/* Indicates the inner headers are valid in the skbuff. */
	__u8			encapsulation:1;
	__u8			encap_hdr_csum:1;
	__u8			csum_valid:1;
#ifdef CONFIG_IPV6_NDISC_NODETYPE
	__u8			ndisc_nodetype:2;
#endif

#if IS_ENABLED(CONFIG_IP_VS)
	__u8			ipvs_property:1;
#endif
#if IS_ENABLED(CONFIG_NETFILTER_XT_TARGET_TRACE) || IS_ENABLED(CONFIG_NF_TABLES)
	__u8			nf_trace:1;
#endif
#ifdef CONFIG_NET_SWITCHDEV
	__u8			offload_fwd_mark:1;
	__u8			offload_l3_fwd_mark:1;
#endif
	__u8			redirected:1;
#ifdef CONFIG_NET_REDIRECT
	__u8			from_ingress:1;
#endif
#ifdef CONFIG_NETFILTER_SKIP_EGRESS
	__u8			nf_skip_egress:1;
#endif
#ifdef CONFIG_TLS_DEVICE
	__u8			decrypted:1;
#endif
	__u8			slow_gro:1;
#if IS_ENABLED(CONFIG_IP_SCTP)
	__u8			csum_not_inet:1;
#endif

#if defined(CONFIG_NET_SCHED) || defined(CONFIG_NET_XGRESS)
	__u16			tc_index;	/* traffic control index */
#endif

	u16			alloc_cpu;

	union {
		__wsum		csum;
		struct {
			__u16	csum_start;
			__u16	csum_offset;
		};
	};
	__u32			priority;
	int			skb_iif;
	__u32			hash;
	union {
		u32		vlan_all;
		struct {
			__be16	vlan_proto;
			__u16	vlan_tci;
		};
	};
#if defined(CONFIG_NET_RX_BUSY_POLL) || defined(CONFIG_XPS)
	union {
		unsigned int	napi_id;
		unsigned int	sender_cpu;
	};
#endif
#ifdef CONFIG_NETWORK_SECMARK
	__u32		secmark;
#endif

	union {
		__u32		mark;
		__u32		reserved_tailroom;
	};

	union {
		__be16		inner_protocol;
		__u8		inner_ipproto;
	};

	__u16			inner_transport_header;
	__u16			inner_network_header;
	__u16			inner_mac_header;

	__be16			protocol;
	__u16			transport_header;
	__u16			network_header;
	__u16			mac_header;

#ifdef CONFIG_KCOV
	u64			kcov_handle;
#endif

	); /* end headers group */

	/* These elements must be at the end, see alloc_skb() for details.  */
	sk_buff_data_t		tail;
	sk_buff_data_t		end;
	unsigned char		*head,
				*data;
	unsigned int		truesize;
	refcount_t		users;

#ifdef CONFIG_SKB_EXTENSIONS
	/* only usable after checking ->active_extensions != 0 */
	struct skb_ext		*extensions;
#endif
};

Output bindings:

#[repr(C)]
pub struct sk_buff {
    pub __bindgen_anon_1: sk_buff__bindgen_ty_1,
    pub sk: *mut sock,
    pub __bindgen_anon_2: sk_buff__bindgen_ty_2,
    pub cb: [core::ffi::c_char; 48usize],
    pub __bindgen_anon_3: sk_buff__bindgen_ty_3,
    pub len: core::ffi::c_uint,
    pub data_len: core::ffi::c_uint,
    pub mac_len: __u16,
    pub hdr_len: __u16,
    pub queue_mapping: __u16,
    pub __cloned_offset: __IncompleteArrayField<__u8>,
    pub _bitfield_align_1: [u8; 0],
    pub _bitfield_1: __BindgenBitfieldUnit<[u8; 1usize]>,
    pub active_extensions: __u8,
    pub __bindgen_anon_4: sk_buff__bindgen_ty_4, // values are in this structure
    pub tail: sk_buff_data_t,
    pub end: sk_buff_data_t,
    pub head: *mut core::ffi::c_uchar,
    pub data: *mut core::ffi::c_uchar,
    pub truesize: core::ffi::c_uint,
    pub users: refcount_t,
    pub extensions: *mut skb_ext,
}
#[repr(C)]
pub struct sk_buff__bindgen_ty_4 {
    pub __bindgen_anon_1: __BindgenUnionField<sk_buff__bindgen_ty_4__bindgen_ty_1>, // one of the values is here
    pub headers: __BindgenUnionField<sk_buff__bindgen_ty_4__bindgen_ty_2>, // and the other one is here
    pub bindgen_union_field: [u32; 14usize],
}
#[repr(C)]
pub struct sk_buff__bindgen_ty_4__bindgen_ty_1 {
    pub __pkt_type_offset: __IncompleteArrayField<__u8>,
    pub _bitfield_align_1: [u8; 0],
    pub _bitfield_1: __BindgenBitfieldUnit<[u8; 1usize]>,
    pub __mono_tc_offset: __IncompleteArrayField<__u8>,
    pub _bitfield_align_2: [u8; 0],
    pub _bitfield_2: __BindgenBitfieldUnit<[u8; 3usize]>,
    pub tc_index: __u16,
    pub alloc_cpu: u16_,
    pub __bindgen_anon_1: sk_buff__bindgen_ty_4__bindgen_ty_1__bindgen_ty_1,
    pub priority: __u32,
    pub skb_iif: core::ffi::c_int,
    pub hash: __u32,
    pub __bindgen_anon_2: sk_buff__bindgen_ty_4__bindgen_ty_1__bindgen_ty_2,
    pub __bindgen_anon_3: sk_buff__bindgen_ty_4__bindgen_ty_1__bindgen_ty_3,
    pub secmark: __u32,
    pub __bindgen_anon_4: sk_buff__bindgen_ty_4__bindgen_ty_1__bindgen_ty_4,
    pub __bindgen_anon_5: sk_buff__bindgen_ty_4__bindgen_ty_1__bindgen_ty_5,
    pub inner_transport_header: __u16,
    pub inner_network_header: __u16,
    pub inner_mac_header: __u16,
    pub protocol: __be16,
    pub transport_header: __u16,
    pub network_header: __u16, // Ladies and gentlemen, we got him
    pub mac_header: __u16,
}
#[repr(C)]
pub struct sk_buff__bindgen_ty_4__bindgen_ty_2 {
    pub __pkt_type_offset: __IncompleteArrayField<__u8>,
    pub _bitfield_align_1: [u8; 0],
    pub _bitfield_1: __BindgenBitfieldUnit<[u8; 1usize]>,
    pub __mono_tc_offset: __IncompleteArrayField<__u8>,
    pub _bitfield_align_2: [u8; 0],
    pub _bitfield_2: __BindgenBitfieldUnit<[u8; 3usize]>,
    pub tc_index: __u16,
    pub alloc_cpu: u16_,
    pub __bindgen_anon_1: sk_buff__bindgen_ty_4__bindgen_ty_2__bindgen_ty_1,
    pub priority: __u32,
    pub skb_iif: core::ffi::c_int,
    pub hash: __u32,
    pub __bindgen_anon_2: sk_buff__bindgen_ty_4__bindgen_ty_2__bindgen_ty_2,
    pub __bindgen_anon_3: sk_buff__bindgen_ty_4__bindgen_ty_2__bindgen_ty_3,
    pub secmark: __u32,
    pub __bindgen_anon_4: sk_buff__bindgen_ty_4__bindgen_ty_2__bindgen_ty_4,
    pub __bindgen_anon_5: sk_buff__bindgen_ty_4__bindgen_ty_2__bindgen_ty_5,
    pub inner_transport_header: __u16,
    pub inner_network_header: __u16,
    pub inner_mac_header: __u16,
    pub protocol: __be16,
    pub transport_header: __u16,
    pub network_header: __u16, // And here it is once again
    pub mac_header: __u16,
}

The structure I was interested in is the __u16 network_header. At the end of the day I was able to get the value (and it seems to be correct) from both places.

I think that this is a bug, since the original structure had the value as the regular field.

Activity

ojeda

ojeda commented on Jan 14, 2025

@ojeda
Contributor

I think that this is a bug, since the original structure had the value as the regular field.

It is not a regular field -- note that it is inside a struct_group, which creates a union of two identical structs, one named and one anonymous.

Reduced:

#define __struct_group(TAG, NAME, ATTRS, MEMBERS...) \
       union { \
               struct { MEMBERS } ATTRS; \
               struct TAG { MEMBERS } ATTRS NAME; \
       }

#define struct_group(NAME, MEMBERS...) \
       __struct_group(/* no tag */, NAME, /* no attrs */, MEMBERS)

typedef unsigned char __u8;
typedef unsigned short __u16;

struct sk_buff {
    struct_group(headers,
    __u8 __pkt_type_offset[0];
    __u16 network_header;
    );
};

If one keeps the __pkt_type_offset, then bindgen uses __IncompleteArrayField and __BindgenUnionField in a struct instead of a union.

Did you see any offsets or other layout data that was incorrect?

MikeRomaniuk

MikeRomaniuk commented on Jan 15, 2025

@MikeRomaniuk
Author

Thanks for answering.
I've missed the struct_group.

Did you see any offsets or other layout data that was incorrect?

It seems everything is ok, so I am closing this issue.

ojeda

ojeda commented on Jan 15, 2025

@ojeda
Contributor

You're welcome, and thanks for reporting!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    rust-for-linuxIssues relevant to the Rust for Linux project

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @ojeda@MikeRomaniuk

        Issue actions

          Incorrect bindings generation with sk_buff C structure from Linux kernel · Issue #3089 · rust-lang/rust-bindgen