net: buf: Pre-initialize user_data on net_buf_alloc()

[jori-nordic]

Introduction

We can initialize a net_buf's user_data member with a known pattern.
Just like the CONFIG_INIT_STACKS option.

Problem description

It is not clear who owns user_data.
Is it the module that allocates it, the API that consumes it, something else?

With a known (non-null) initialized value, it will be possible to perform some checks and avoid head-scratching debug sessions. We've had some of those sessions in Bluetooth/L2CAP.

Proposed change

Call memset(buf->user_data, 0xAA, buf->user_data_size) in net_buf_alloc() right before returning buf.
Can be locked behind a kconfig, just like the stacks option.

Concerns and Unresolved Questions

Other users (maybe out of tree too) could break due to NULL checks on user_data not working anymore.
E.g. the pattern of casting cb from user_data and having a block like this one:

if (cb) {
  cb(some_param);
}

Alternatives

• APIs that consume net_bufs enforce clearing by the application

[jori-nordic]

proposed by @sjanc here: https://discord.com/channels/720317445772017664/733036837576376341/1273296703868633241

Due to me breaking bt_l2cap_chan_send() API and not mentioning it in the docstring. I'll submit a PR ASAP.

[Thalley]

Disclaimer: Not claiming that we need to do a major refactor or significantly modify our API

My thoughts about user_data has always been that it's the owner of the buffer that controls that value. For example when application wants to use the L2CAP API to send some data via e.g. bt_l2cap_chan_send, then the buf->user_data should belong to whomever owns the buffer. After all, it's the owner of the buffer that allocates the buffer and userdata with e.g. NET_BUF_POOL_FIXED_DEFINE.

In that train of thought it should perhaps have been owner_data instead of user_data, as the user of the data is often not the owner (like in L2CAP if I understand correctly).

I'm not opposed to applying a patch to prevent issue and keep on with the current API, as I believe is suggested here

[sjanc]

note: this is affecting multiple bluetooth qualification tests (I'd consider reverting check for time being)

[jori-nordic]

Ah okay. Then you see, the stack's way of thinking is the opposite (for which I made the check). It used user_data to put internal stack data inside (callback, etc).

In the previous meeting, @jhedberg showed us the linux kernel's skbuff, which was the inspiration for net_buf in Zephyr. It specifically says the user_data is the allocator's and definitely not intended for passing data between layers.

We mostly use user_data for passing data between layers for in the Bluetooth stack. So maybe we need to change that and not use user_data at all.

[jori-nordic]

consider reverting check for time being

Made a PR to downgrade to a warning: #77113

[jori-nordic]

Bluetooth WG:

• Consensus is to unconditionally memset to 0 in net_buf_alloc(). @jori-nordic will make PR.
• Might even decrease code size, as clearing will be centralized and not duplicated across users