On Fri, Aug 13, 2021 at 09:40:07AM +0200, Johannes Berg wrote:

On Sat, 2021-07-31 at 08:55 -0700, Kees Cook wrote:

quoted

On Tue, Jul 27, 2021 at 01:58:30PM -0700, Kees Cook wrote:

quoted

In preparation for FORTIFY_SOURCE performing compile-time and run-time
field bounds checking for memset(), avoid intentionally writing across
neighboring fields.

Use memset_after() so memset() doesn't get confused about writing
beyond the destination member that is intended to be the starting point
of zeroing through the end of the struct.

Note that the common helper, ieee80211_tx_info_clear_status(), does NOT
clear ack_signal, but the open-coded versions do. All three perform
checks that the ack_signal position hasn't changed, though.

Quick ping on this question: there is a mismatch between the common
helper and the other places that do this. Is there a bug here?

Yes.

The common helper should also clear ack_signal, but that was broken by
commit e3e1a0bcb3f1 ("mac80211: reduce IEEE80211_TX_MAX_RATES"), because
that commit changed the order of the fields and updated carl9170 and p54
properly but not the common helper...

It looks like p54 actually uses the rates, which is why it does this
manually. I can't see why carl9170 does this manually, though.

It doesn't actually matter much because ack_signal is normally filled in
afterwards, and even if it isn't, it's just for statistics.

The correct thing to do here would be to

	memset_after(&info->status, 0, rates);

Sounds good; I will adjust these (and drop the BULID_BUG_ONs, as you
suggest in the next email).

Thanks!

-Kees

-- 
Kees Cook