| Message ID | 20210617042709.2170111-1-keescook@chromium.org (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | mac80211: Recast pointer for trailing memcpy() | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Guessed tree name to be net-next |
| netdev/subject_prefix | warning | Target tree name not specified in the subject |
| netdev/cc_maintainers | success | CCed 5 of 5 maintainers |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 50 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
On Wed, Jun 16, 2021 at 09:27:09PM -0700, Kees Cook wrote: > In preparation for FORTIFY_SOURCE performing compile-time and run-time > field bounds checking for memcpy(), memmove(), and memset(), avoid > intentionally writing across neighboring array fields. > > Give memcpy() a specific source pointer type so it can correctly > calculate the bounds of the copy. Hmpf, please ignore this patch; sorry for the noise. This fix got mis-tested on my end and does not solve the problem I was trying to solve. I will return with a v2. :)
From: Kees Cook > Sent: 17 June 2021 05:27 > > In preparation for FORTIFY_SOURCE performing compile-time and run-time > field bounds checking for memcpy(), memmove(), and memset(), avoid > intentionally writing across neighboring array fields. > > Give memcpy() a specific source pointer type so it can correctly > calculate the bounds of the copy. Doesn't the necessity of this sort of patch just sidestep the run-time checking and really indicate that it is just a complete waste of cpu resources? I bet code changes to avoid/fix the reported errors will introduce more bugs than the test itself will really find. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c index af0ef456eb0f..cb141bb788a8 100644 --- a/net/mac80211/rx.c +++ b/net/mac80211/rx.c @@ -559,6 +559,8 @@ ieee80211_add_rx_radiotap_header(struct ieee80211_local *local, if (status->encoding == RX_ENC_HE && status->flag & RX_FLAG_RADIOTAP_HE) { + struct ieee80211_radiotap_he *he_ptr; + #define HE_PREP(f, val) le16_encode_bits(val, IEEE80211_RADIOTAP_HE_##f) if (status->enc_flags & RX_ENC_FLAG_STBC_MASK) { @@ -626,18 +628,22 @@ ieee80211_add_rx_radiotap_header(struct ieee80211_local *local, while ((pos - (u8 *)rthdr) & 1) pos++; rthdr->it_present |= cpu_to_le32(1 << IEEE80211_RADIOTAP_HE); - memcpy(pos, &he, sizeof(he)); - pos += sizeof(he); + he_ptr = (void *)pos; + memcpy(he_ptr, &he, sizeof(he)); + pos += sizeof(*he_ptr); } if (status->encoding == RX_ENC_HE && status->flag & RX_FLAG_RADIOTAP_HE_MU) { + struct ieee80211_radiotap_he_mu *he_mu_ptr; + /* ensure 2 byte alignment */ while ((pos - (u8 *)rthdr) & 1) pos++; rthdr->it_present |= cpu_to_le32(1 << IEEE80211_RADIOTAP_HE_MU); - memcpy(pos, &he_mu, sizeof(he_mu)); - pos += sizeof(he_mu); + he_mu_ptr = (void *)pos; + memcpy(he_mu_ptr, &he_mu, sizeof(he_mu)); + pos += sizeof(*he_mu_ptr); } if (status->flag & RX_FLAG_NO_PSDU) { @@ -647,12 +653,14 @@ ieee80211_add_rx_radiotap_header(struct ieee80211_local *local, } if (status->flag & RX_FLAG_RADIOTAP_LSIG) { + struct ieee80211_radiotap_lsig *lsig_ptr; /* ensure 2 byte alignment */ while ((pos - (u8 *)rthdr) & 1) pos++; rthdr->it_present |= cpu_to_le32(1 << IEEE80211_RADIOTAP_LSIG); - memcpy(pos, &lsig, sizeof(lsig)); - pos += sizeof(lsig); + lsig_ptr = (void *)pos; + memcpy(lsig_ptr, &lsig, sizeof(lsig)); + pos += sizeof(*lsig_ptr); } for_each_set_bit(chain, &chains, IEEE80211_MAX_CHAINS) {
In preparation for FORTIFY_SOURCE performing compile-time and run-time field bounds checking for memcpy(), memmove(), and memset(), avoid intentionally writing across neighboring array fields. Give memcpy() a specific source pointer type so it can correctly calculate the bounds of the copy. Signed-off-by: Kees Cook <keescook@chromium.org> --- net/mac80211/rx.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-)