| Message ID | 20220812055249.8037-1-palmer@rivosinc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Bluetooth: L2CAP: Elide a string overflow warning | expand |
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=667062
---Test result---
Test Summary:
CheckPatch FAIL 1.60 seconds
GitLint FAIL 1.01 seconds
SubjectPrefix PASS 0.84 seconds
BuildKernel PASS 41.49 seconds
BuildKernel32 PASS 34.89 seconds
Incremental Build with patchesPASS 50.21 seconds
TestRunner: Setup PASS 605.56 seconds
TestRunner: l2cap-tester PASS 20.02 seconds
TestRunner: bnep-tester PASS 8.10 seconds
TestRunner: mgmt-tester PASS 122.17 seconds
TestRunner: rfcomm-tester PASS 11.73 seconds
TestRunner: sco-tester PASS 11.48 seconds
TestRunner: smp-tester PASS 12.14 seconds
TestRunner: userchan-tester PASS 8.96 seconds
Details
##############################
Test: CheckPatch - FAIL - 1.60 seconds
Run checkpatch.pl script with rule in .checkpatch.conf
Bluetooth: L2CAP: Elide a string overflow warning\WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#88:
In file included from /scratch/merges/ko-linux-next/linux/include/linux/string.h:253,
WARNING:USE_RELATIVE_PATH: use relative pathname instead of absolute in changelog text
#95:
from /scratch/merges/ko-linux-next/linux/net/bluetooth/l2cap_core.c:31:
WARNING:USE_RELATIVE_PATH: use relative pathname instead of absolute in changelog text
#98:
inlined from 'l2cap_global_chan_by_psm' at /scratch/merges/ko-linux-next/linux/net/bluetooth/l2cap_core.c:2003:15:
WARNING:USE_RELATIVE_PATH: use relative pathname instead of absolute in changelog text
#99:
/scratch/merges/ko-linux-next/linux/include/linux/fortify-string.h:44:33: error: '__builtin_memcmp' specified bound 6 exceeds source size 0 [-Werror=stringop-overread]
WARNING:USE_RELATIVE_PATH: use relative pathname instead of absolute in changelog text
#102:
/scratch/merges/ko-linux-next/linux/include/linux/fortify-string.h:420:16: note: in expansion of macro '__underlying_memcmp'
WARNING:USE_RELATIVE_PATH: use relative pathname instead of absolute in changelog text
#107:
inlined from 'l2cap_global_chan_by_psm' at /scratch/merges/ko-linux-next/linux/net/bluetooth/l2cap_core.c:2004:15:
WARNING:USE_RELATIVE_PATH: use relative pathname instead of absolute in changelog text
#108:
/scratch/merges/ko-linux-next/linux/include/linux/fortify-string.h:44:33: error: '__builtin_memcmp' specified bound 6 exceeds source size 0 [-Werror=stringop-overread]
WARNING:USE_RELATIVE_PATH: use relative pathname instead of absolute in changelog text
#111:
/scratch/merges/ko-linux-next/linux/include/linux/fortify-string.h:420:16: note: in expansion of macro '__underlying_memcmp'
total: 0 errors, 8 warnings, 0 checks, 18 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
/github/workspace/src/12941909.patch has style problems, please review.
NOTE: Ignored message types: UNKNOWN_COMMIT_ID
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
##############################
Test: GitLint - FAIL - 1.01 seconds
Run gitlint with rule in .gitlint
Bluetooth: L2CAP: Elide a string overflow warning
9: B1 Line exceeds max length (85>80): "In file included from /scratch/merges/ko-linux-next/linux/include/linux/string.h:253,"
10: B1 Line exceeds max length (84>80): " from /scratch/merges/ko-linux-next/linux/include/linux/bitmap.h:11,"
11: B1 Line exceeds max length (85>80): " from /scratch/merges/ko-linux-next/linux/include/linux/cpumask.h:12,"
12: B1 Line exceeds max length (91>80): " from /scratch/merges/ko-linux-next/linux/include/linux/mm_types_task.h:14,"
13: B1 Line exceeds max length (85>80): " from /scratch/merges/ko-linux-next/linux/include/linux/mm_types.h:5,"
14: B1 Line exceeds max length (84>80): " from /scratch/merges/ko-linux-next/linux/include/linux/buildid.h:5,"
15: B1 Line exceeds max length (84>80): " from /scratch/merges/ko-linux-next/linux/include/linux/module.h:14,"
16: B1 Line exceeds max length (88>80): " from /scratch/merges/ko-linux-next/linux/net/bluetooth/l2cap_core.c:31:"
18: B1 Line exceeds max length (104>80): " inlined from 'bacmp' at /scratch/merges/ko-linux-next/linux/include/net/bluetooth/bluetooth.h:347:9,"
19: B1 Line exceeds max length (118>80): " inlined from 'l2cap_global_chan_by_psm' at /scratch/merges/ko-linux-next/linux/net/bluetooth/l2cap_core.c:2003:15:"
20: B1 Line exceeds max length (167>80): "/scratch/merges/ko-linux-next/linux/include/linux/fortify-string.h:44:33: error: '__builtin_memcmp' specified bound 6 exceeds source size 0 [-Werror=stringop-overread]"
23: B1 Line exceeds max length (124>80): "/scratch/merges/ko-linux-next/linux/include/linux/fortify-string.h:420:16: note: in expansion of macro '__underlying_memcmp'"
27: B1 Line exceeds max length (104>80): " inlined from 'bacmp' at /scratch/merges/ko-linux-next/linux/include/net/bluetooth/bluetooth.h:347:9,"
28: B1 Line exceeds max length (118>80): " inlined from 'l2cap_global_chan_by_psm' at /scratch/merges/ko-linux-next/linux/net/bluetooth/l2cap_core.c:2004:15:"
29: B1 Line exceeds max length (167>80): "/scratch/merges/ko-linux-next/linux/include/linux/fortify-string.h:44:33: error: '__builtin_memcmp' specified bound 6 exceeds source size 0 [-Werror=stringop-overread]"
32: B1 Line exceeds max length (124>80): "/scratch/merges/ko-linux-next/linux/include/linux/fortify-string.h:420:16: note: in expansion of macro '__underlying_memcmp'"
---
Regards,
Linux Bluetooth
Hi Palmer, On Fri, Aug 12, 2022 at 12:19 AM <bluez.test.bot@gmail.com> wrote: > > This is automated email and please do not reply to this email! > > Dear submitter, > > Thank you for submitting the patches to the linux bluetooth mailing list. > This is a CI test results with your patch series: > PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=667062 > > ---Test result--- > > Test Summary: > CheckPatch FAIL 1.60 seconds > GitLint FAIL 1.01 seconds > SubjectPrefix PASS 0.84 seconds > BuildKernel PASS 41.49 seconds > BuildKernel32 PASS 34.89 seconds > Incremental Build with patchesPASS 50.21 seconds > TestRunner: Setup PASS 605.56 seconds > TestRunner: l2cap-tester PASS 20.02 seconds > TestRunner: bnep-tester PASS 8.10 seconds > TestRunner: mgmt-tester PASS 122.17 seconds > TestRunner: rfcomm-tester PASS 11.73 seconds > TestRunner: sco-tester PASS 11.48 seconds > TestRunner: smp-tester PASS 12.14 seconds > TestRunner: userchan-tester PASS 8.96 seconds > > Details > ############################## > Test: CheckPatch - FAIL - 1.60 seconds > Run checkpatch.pl script with rule in .checkpatch.conf > Bluetooth: L2CAP: Elide a string overflow warning\WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line) > #88: > In file included from /scratch/merges/ko-linux-next/linux/include/linux/string.h:253, > > WARNING:USE_RELATIVE_PATH: use relative pathname instead of absolute in changelog text > #95: > from /scratch/merges/ko-linux-next/linux/net/bluetooth/l2cap_core.c:31: > > WARNING:USE_RELATIVE_PATH: use relative pathname instead of absolute in changelog text > #98: > inlined from 'l2cap_global_chan_by_psm' at /scratch/merges/ko-linux-next/linux/net/bluetooth/l2cap_core.c:2003:15: > > WARNING:USE_RELATIVE_PATH: use relative pathname instead of absolute in changelog text > #99: > /scratch/merges/ko-linux-next/linux/include/linux/fortify-string.h:44:33: error: '__builtin_memcmp' specified bound 6 exceeds source size 0 [-Werror=stringop-overread] > > WARNING:USE_RELATIVE_PATH: use relative pathname instead of absolute in changelog text > #102: > /scratch/merges/ko-linux-next/linux/include/linux/fortify-string.h:420:16: note: in expansion of macro '__underlying_memcmp' > > WARNING:USE_RELATIVE_PATH: use relative pathname instead of absolute in changelog text > #107: > inlined from 'l2cap_global_chan_by_psm' at /scratch/merges/ko-linux-next/linux/net/bluetooth/l2cap_core.c:2004:15: > > WARNING:USE_RELATIVE_PATH: use relative pathname instead of absolute in changelog text > #108: > /scratch/merges/ko-linux-next/linux/include/linux/fortify-string.h:44:33: error: '__builtin_memcmp' specified bound 6 exceeds source size 0 [-Werror=stringop-overread] > > WARNING:USE_RELATIVE_PATH: use relative pathname instead of absolute in changelog text > #111: > /scratch/merges/ko-linux-next/linux/include/linux/fortify-string.h:420:16: note: in expansion of macro '__underlying_memcmp' > > total: 0 errors, 8 warnings, 0 checks, 18 lines checked > > NOTE: For some of the reported defects, checkpatch may be able to > mechanically convert to the typical style using --fix or --fix-inplace. > > /github/workspace/src/12941909.patch has style problems, please review. > > NOTE: Ignored message types: UNKNOWN_COMMIT_ID > > NOTE: If any of the errors are false positives, please report > them to the maintainer, see CHECKPATCH in MAINTAINERS. > > > ############################## > Test: GitLint - FAIL - 1.01 seconds > Run gitlint with rule in .gitlint > Bluetooth: L2CAP: Elide a string overflow warning > 9: B1 Line exceeds max length (85>80): "In file included from /scratch/merges/ko-linux-next/linux/include/linux/string.h:253," > 10: B1 Line exceeds max length (84>80): " from /scratch/merges/ko-linux-next/linux/include/linux/bitmap.h:11," > 11: B1 Line exceeds max length (85>80): " from /scratch/merges/ko-linux-next/linux/include/linux/cpumask.h:12," > 12: B1 Line exceeds max length (91>80): " from /scratch/merges/ko-linux-next/linux/include/linux/mm_types_task.h:14," > 13: B1 Line exceeds max length (85>80): " from /scratch/merges/ko-linux-next/linux/include/linux/mm_types.h:5," > 14: B1 Line exceeds max length (84>80): " from /scratch/merges/ko-linux-next/linux/include/linux/buildid.h:5," > 15: B1 Line exceeds max length (84>80): " from /scratch/merges/ko-linux-next/linux/include/linux/module.h:14," > 16: B1 Line exceeds max length (88>80): " from /scratch/merges/ko-linux-next/linux/net/bluetooth/l2cap_core.c:31:" > 18: B1 Line exceeds max length (104>80): " inlined from 'bacmp' at /scratch/merges/ko-linux-next/linux/include/net/bluetooth/bluetooth.h:347:9," > 19: B1 Line exceeds max length (118>80): " inlined from 'l2cap_global_chan_by_psm' at /scratch/merges/ko-linux-next/linux/net/bluetooth/l2cap_core.c:2003:15:" > 20: B1 Line exceeds max length (167>80): "/scratch/merges/ko-linux-next/linux/include/linux/fortify-string.h:44:33: error: '__builtin_memcmp' specified bound 6 exceeds source size 0 [-Werror=stringop-overread]" > 23: B1 Line exceeds max length (124>80): "/scratch/merges/ko-linux-next/linux/include/linux/fortify-string.h:420:16: note: in expansion of macro '__underlying_memcmp'" > 27: B1 Line exceeds max length (104>80): " inlined from 'bacmp' at /scratch/merges/ko-linux-next/linux/include/net/bluetooth/bluetooth.h:347:9," > 28: B1 Line exceeds max length (118>80): " inlined from 'l2cap_global_chan_by_psm' at /scratch/merges/ko-linux-next/linux/net/bluetooth/l2cap_core.c:2004:15:" > 29: B1 Line exceeds max length (167>80): "/scratch/merges/ko-linux-next/linux/include/linux/fortify-string.h:44:33: error: '__builtin_memcmp' specified bound 6 exceeds source size 0 [-Werror=stringop-overread]" > 32: B1 Line exceeds max length (124>80): "/scratch/merges/ko-linux-next/linux/include/linux/fortify-string.h:420:16: note: in expansion of macro '__underlying_memcmp'" How about we attempt to fix the root cause in bacmp: diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h index e72f3b247b5e..415a5f3afc98 100644 --- a/include/net/bluetooth/bluetooth.h +++ b/include/net/bluetooth/bluetooth.h @@ -344,6 +344,9 @@ static inline bool bdaddr_type_is_le(u8 type) /* Copy, swap, convert BD Address */ static inline int bacmp(const bdaddr_t *ba1, const bdaddr_t *ba2) { + if (!ba1 || !ba2) + return -EINVAL; + return memcmp(ba1, ba2, sizeof(bdaddr_t)); } static inline void bacpy(bdaddr_t *dst, const bdaddr_t *src) We could alternatively make it compare the pointer first so in case both are NULL it returns 0 but since memcmp itself causes warnings if NULL is passed I guess it safe to assume NULL pointers shall be considered considered invalid, that said perhaps the problem is if one would consider -EINVAL a return of memcmp so returning negative values becomes ambiguous but the original intent of bacmp was to compare bluetooth addresses so users normally don't check what the error actually means and instead just use its return as a bool.
On Fri, 12 Aug 2022 11:08:35 PDT (-0700), luiz.dentz@gmail.com wrote: > Hi Palmer, > > On Fri, Aug 12, 2022 at 12:19 AM <bluez.test.bot@gmail.com> wrote: >> >> This is automated email and please do not reply to this email! >> >> Dear submitter, >> >> Thank you for submitting the patches to the linux bluetooth mailing list. >> This is a CI test results with your patch series: >> PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=667062 >> >> ---Test result--- >> >> Test Summary: >> CheckPatch FAIL 1.60 seconds >> GitLint FAIL 1.01 seconds >> SubjectPrefix PASS 0.84 seconds >> BuildKernel PASS 41.49 seconds >> BuildKernel32 PASS 34.89 seconds >> Incremental Build with patchesPASS 50.21 seconds >> TestRunner: Setup PASS 605.56 seconds >> TestRunner: l2cap-tester PASS 20.02 seconds >> TestRunner: bnep-tester PASS 8.10 seconds >> TestRunner: mgmt-tester PASS 122.17 seconds >> TestRunner: rfcomm-tester PASS 11.73 seconds >> TestRunner: sco-tester PASS 11.48 seconds >> TestRunner: smp-tester PASS 12.14 seconds >> TestRunner: userchan-tester PASS 8.96 seconds >> >> Details >> ############################## >> Test: CheckPatch - FAIL - 1.60 seconds >> Run checkpatch.pl script with rule in .checkpatch.conf >> Bluetooth: L2CAP: Elide a string overflow warning\WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line) >> #88: >> In file included from /scratch/merges/ko-linux-next/linux/include/linux/string.h:253, >> >> WARNING:USE_RELATIVE_PATH: use relative pathname instead of absolute in changelog text >> #95: >> from /scratch/merges/ko-linux-next/linux/net/bluetooth/l2cap_core.c:31: >> >> WARNING:USE_RELATIVE_PATH: use relative pathname instead of absolute in changelog text >> #98: >> inlined from 'l2cap_global_chan_by_psm' at /scratch/merges/ko-linux-next/linux/net/bluetooth/l2cap_core.c:2003:15: >> >> WARNING:USE_RELATIVE_PATH: use relative pathname instead of absolute in changelog text >> #99: >> /scratch/merges/ko-linux-next/linux/include/linux/fortify-string.h:44:33: error: '__builtin_memcmp' specified bound 6 exceeds source size 0 [-Werror=stringop-overread] >> >> WARNING:USE_RELATIVE_PATH: use relative pathname instead of absolute in changelog text >> #102: >> /scratch/merges/ko-linux-next/linux/include/linux/fortify-string.h:420:16: note: in expansion of macro '__underlying_memcmp' >> >> WARNING:USE_RELATIVE_PATH: use relative pathname instead of absolute in changelog text >> #107: >> inlined from 'l2cap_global_chan_by_psm' at /scratch/merges/ko-linux-next/linux/net/bluetooth/l2cap_core.c:2004:15: >> >> WARNING:USE_RELATIVE_PATH: use relative pathname instead of absolute in changelog text >> #108: >> /scratch/merges/ko-linux-next/linux/include/linux/fortify-string.h:44:33: error: '__builtin_memcmp' specified bound 6 exceeds source size 0 [-Werror=stringop-overread] >> >> WARNING:USE_RELATIVE_PATH: use relative pathname instead of absolute in changelog text >> #111: >> /scratch/merges/ko-linux-next/linux/include/linux/fortify-string.h:420:16: note: in expansion of macro '__underlying_memcmp' >> >> total: 0 errors, 8 warnings, 0 checks, 18 lines checked >> >> NOTE: For some of the reported defects, checkpatch may be able to >> mechanically convert to the typical style using --fix or --fix-inplace. >> >> /github/workspace/src/12941909.patch has style problems, please review. >> >> NOTE: Ignored message types: UNKNOWN_COMMIT_ID >> >> NOTE: If any of the errors are false positives, please report >> them to the maintainer, see CHECKPATCH in MAINTAINERS. >> >> >> ############################## >> Test: GitLint - FAIL - 1.01 seconds >> Run gitlint with rule in .gitlint >> Bluetooth: L2CAP: Elide a string overflow warning >> 9: B1 Line exceeds max length (85>80): "In file included from /scratch/merges/ko-linux-next/linux/include/linux/string.h:253," >> 10: B1 Line exceeds max length (84>80): " from /scratch/merges/ko-linux-next/linux/include/linux/bitmap.h:11," >> 11: B1 Line exceeds max length (85>80): " from /scratch/merges/ko-linux-next/linux/include/linux/cpumask.h:12," >> 12: B1 Line exceeds max length (91>80): " from /scratch/merges/ko-linux-next/linux/include/linux/mm_types_task.h:14," >> 13: B1 Line exceeds max length (85>80): " from /scratch/merges/ko-linux-next/linux/include/linux/mm_types.h:5," >> 14: B1 Line exceeds max length (84>80): " from /scratch/merges/ko-linux-next/linux/include/linux/buildid.h:5," >> 15: B1 Line exceeds max length (84>80): " from /scratch/merges/ko-linux-next/linux/include/linux/module.h:14," >> 16: B1 Line exceeds max length (88>80): " from /scratch/merges/ko-linux-next/linux/net/bluetooth/l2cap_core.c:31:" >> 18: B1 Line exceeds max length (104>80): " inlined from 'bacmp' at /scratch/merges/ko-linux-next/linux/include/net/bluetooth/bluetooth.h:347:9," >> 19: B1 Line exceeds max length (118>80): " inlined from 'l2cap_global_chan_by_psm' at /scratch/merges/ko-linux-next/linux/net/bluetooth/l2cap_core.c:2003:15:" >> 20: B1 Line exceeds max length (167>80): "/scratch/merges/ko-linux-next/linux/include/linux/fortify-string.h:44:33: error: '__builtin_memcmp' specified bound 6 exceeds source size 0 [-Werror=stringop-overread]" >> 23: B1 Line exceeds max length (124>80): "/scratch/merges/ko-linux-next/linux/include/linux/fortify-string.h:420:16: note: in expansion of macro '__underlying_memcmp'" >> 27: B1 Line exceeds max length (104>80): " inlined from 'bacmp' at /scratch/merges/ko-linux-next/linux/include/net/bluetooth/bluetooth.h:347:9," >> 28: B1 Line exceeds max length (118>80): " inlined from 'l2cap_global_chan_by_psm' at /scratch/merges/ko-linux-next/linux/net/bluetooth/l2cap_core.c:2004:15:" >> 29: B1 Line exceeds max length (167>80): "/scratch/merges/ko-linux-next/linux/include/linux/fortify-string.h:44:33: error: '__builtin_memcmp' specified bound 6 exceeds source size 0 [-Werror=stringop-overread]" >> 32: B1 Line exceeds max length (124>80): "/scratch/merges/ko-linux-next/linux/include/linux/fortify-string.h:420:16: note: in expansion of macro '__underlying_memcmp'" > > How about we attempt to fix the root cause in bacmp: > > diff --git a/include/net/bluetooth/bluetooth.h > b/include/net/bluetooth/bluetooth.h > index e72f3b247b5e..415a5f3afc98 100644 > --- a/include/net/bluetooth/bluetooth.h > +++ b/include/net/bluetooth/bluetooth.h > @@ -344,6 +344,9 @@ static inline bool bdaddr_type_is_le(u8 type) > /* Copy, swap, convert BD Address */ > static inline int bacmp(const bdaddr_t *ba1, const bdaddr_t *ba2) > { > + if (!ba1 || !ba2) > + return -EINVAL; > + > return memcmp(ba1, ba2, sizeof(bdaddr_t)); > } > static inline void bacpy(bdaddr_t *dst, const bdaddr_t *src) > > We could alternatively make it compare the pointer first so in case > both are NULL it returns 0 but since memcmp itself causes warnings if > NULL is passed I guess it safe to assume NULL pointers shall be > considered considered invalid, that said perhaps the problem is if one > would consider -EINVAL a return of memcmp so returning negative values > becomes ambiguous but the original intent of bacmp was to compare > bluetooth addresses so users normally don't check what the error > actually means and instead just use its return as a bool. I'm fine with that, but just from looking at the bacmp() callers it seems like many of them would need to be changed to handle the -EINVAL as they're assuming it looks like memcmp(). IIUC the -EINVAL aliases with possible valid memcmp() returns, so it's not super clear how to adjust the callers. I guess none of them could have null inputs now as otherwise it'd crash so maybe that doesn't matter? I'm not really a bluethooth guy, but LMK if you want me to take a shot at it.
On Fri, 12 Aug 2022 11:22:49 +0530 Palmer Dabbelt wrote: > From: Palmer Dabbelt <palmer@rivosinc.com> > > Without this I get a string op warning related to copying from a > possibly NULL pointer. I think the warning is spurious, but it's > tripping up allmodconfig. I think it is not spurious, and is due to the following commit: d0be8347c623 ("Bluetooth: L2CAP: Fix use-after-free caused by l2cap_chan_put") The following commit fixes a similar problem (added the NULL check on line 1996): 332f1795ca20 ("Bluetooth: L2CAP: Fix l2cap_global_chan_by_psm regression") > In file included from /scratch/merges/ko-linux-next/linux/include/linux/string.h:253, > from /scratch/merges/ko-linux-next/linux/include/linux/bitmap.h:11, > from /scratch/merges/ko-linux-next/linux/include/linux/cpumask.h:12, > from /scratch/merges/ko-linux-next/linux/include/linux/mm_types_task.h:14, > from /scratch/merges/ko-linux-next/linux/include/linux/mm_types.h:5, > from /scratch/merges/ko-linux-next/linux/include/linux/buildid.h:5, > from /scratch/merges/ko-linux-next/linux/include/linux/module.h:14, > from /scratch/merges/ko-linux-next/linux/net/bluetooth/l2cap_core.c:31: > In function 'memcmp', > inlined from 'bacmp' at /scratch/merges/ko-linux-next/linux/include/net/bluetooth/bluetooth.h:347:9, > inlined from 'l2cap_global_chan_by_psm' at /scratch/merges/ko-linux-next/linux/net/bluetooth/l2cap_core.c:2003:15: > /scratch/merges/ko-linux-next/linux/include/linux/fortify-string.h:44:33: error: '__builtin_memcmp' specified bound 6 exceeds source size 0 [-Werror=stringop-overread] > 44 | #define __underlying_memcmp __builtin_memcmp > | ^ > /scratch/merges/ko-linux-next/linux/include/linux/fortify-string.h:420:16: note: in expansion of macro '__underlying_memcmp' > 420 | return __underlying_memcmp(p, q, size); > | ^~~~~~~~~~~~~~~~~~~ > In function 'memcmp', > inlined from 'bacmp' at /scratch/merges/ko-linux-next/linux/include/net/bluetooth/bluetooth.h:347:9, > inlined from 'l2cap_global_chan_by_psm' at /scratch/merges/ko-linux-next/linux/net/bluetooth/l2cap_core.c:2004:15: > /scratch/merges/ko-linux-next/linux/include/linux/fortify-string.h:44:33: error: '__builtin_memcmp' specified bound 6 exceeds source size 0 [-Werror=stringop-overread] > 44 | #define __underlying_memcmp __builtin_memcmp > | ^ > /scratch/merges/ko-linux-next/linux/include/linux/fortify-string.h:420:16: note: in expansion of macro '__underlying_memcmp' > 420 | return __underlying_memcmp(p, q, size); > | ^~~~~~~~~~~~~~~~~~~ > cc1: all warnings being treated as errors > > Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com> Tested-by: Siddh Raman Pant <code@siddh.me> > --- > net/bluetooth/l2cap_core.c | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c > index cbe0cae73434..be7f47e52119 100644 > --- a/net/bluetooth/l2cap_core.c > +++ b/net/bluetooth/l2cap_core.c > @@ -2000,11 +2000,13 @@ static struct l2cap_chan *l2cap_global_chan_by_psm(int state, __le16 psm, > } > > /* Closest match */ > - src_any = !bacmp(&c->src, BDADDR_ANY); > - dst_any = !bacmp(&c->dst, BDADDR_ANY); > - if ((src_match && dst_any) || (src_any && dst_match) || > - (src_any && dst_any)) > - c1 = c; > + if (c) { > + src_any = !bacmp(&c->src, BDADDR_ANY); > + dst_any = !bacmp(&c->dst, BDADDR_ANY); > + if ((src_match && dst_any) || (src_any && dst_match) || > + (src_any && dst_any)) > + c1 = c; > + } > } > } >
> -----Original Message----- > From: Siddh Raman Pant <code@siddh.me> > Sent: Thursday, August 25, 2022 6:01 AM > To: palmer@rivosinc.com > Cc: davem@davemloft.net; edumazet@google.com; johan.hedberg@gmail.com; > kuba@kernel.org; linux-bluetooth@vger.kernel.org; linux- > kernel@vger.kernel.org; linux@rivosinc.com; luiz.dentz@gmail.com; > marcel@holtmann.org; netdev@vger.kernel.org; pabeni@redhat.com > Subject: Re: [PATCH] Bluetooth: L2CAP: Elide a string overflow warning > > On Fri, 12 Aug 2022 11:22:49 +0530 Palmer Dabbelt wrote: > > From: Palmer Dabbelt <palmer@rivosinc.com> > > > > Without this I get a string op warning related to copying from a > > possibly NULL pointer. I think the warning is spurious, but it's > > tripping up allmodconfig. > > I think it is not spurious, and is due to the following commit: > d0be8347c623 ("Bluetooth: L2CAP: Fix use-after-free caused by l2cap_chan_put") That commit was OK - it added an "if (!c) continue" to handle if the value c is changed to NULL. > The following commit fixes a similar problem (added the NULL check on line > 1996): > 332f1795ca20 ("Bluetooth: L2CAP: Fix l2cap_global_chan_by_psm regression") That commit wiped out the "if (!c) continue" path escape clause from the previous patch, introducing a path back to code that doesn't check for NULL: - if (!c) - continue; - - read_unlock(&chan_list_lock); - return c; + if (c) { + read_unlock(&chan_list_lock); + return c; + } } src_any = !bacmp(&c->src, BDADDR_ANY); dst_any = !bacmp(&c->dst, BDADDR_ANY); > > inlined from 'l2cap_global_chan_by_psm' at /scratch/merges/ko-linux- > next/linux/net/bluetooth/l2cap_core.c:2003:15: > > /scratch/merges/ko-linux-next/linux/include/linux/fortify-string.h:44:33: > error: '__builtin_memcmp' specified bound 6 exceeds source size 0 [- > Werror=stringop-overread] > > 44 | #define __underlying_memcmp __builtin_memcmp > > | ^ > > /scratch/merges/ko-linux-next/linux/include/linux/fortify-string.h:420:16: > note: in expansion of macro '__underlying_memcmp' > > 420 | return __underlying_memcmp(p, q, size); > > | ^~~~~~~~~~~~~~~~~~~ > > In function 'memcmp', > > inlined from 'bacmp' at /scratch/merges/ko-linux- > next/linux/include/net/bluetooth/bluetooth.h:347:9, > > inlined from 'l2cap_global_chan_by_psm' at /scratch/merges/ko-linux- ... > > > > Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com> > > Tested-by: Siddh Raman Pant <code@siddh.me> This patch is necessary to get a successful cross-compile of 6.0-rc3 allmodconfig for ARCH=mips on x86. Tested-by: Robert Elliott <elliott@hpe.com> I suggest you label this patch as: Fixes: 332f1795ca20 ("Bluetooth: L2CAP: Fix l2cap_global_chan_by_psm regression") > > --- > > net/bluetooth/l2cap_core.c | 12 +++++++----- > > 1 file changed, 7 insertions(+), 5 deletions(-) > > > > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c > > index cbe0cae73434..be7f47e52119 100644 > > --- a/net/bluetooth/l2cap_core.c > > +++ b/net/bluetooth/l2cap_core.c > > @@ -2000,11 +2000,13 @@ static struct l2cap_chan > *l2cap_global_chan_by_psm(int state, __le16 psm, > > } > > > > /* Closest match */ > > - src_any = !bacmp(&c->src, BDADDR_ANY); > > - dst_any = !bacmp(&c->dst, BDADDR_ANY); > > - if ((src_match && dst_any) || (src_any && dst_match) || > > - (src_any && dst_any)) > > - c1 = c; > > + if (c) { > > + src_any = !bacmp(&c->src, BDADDR_ANY); > > + dst_any = !bacmp(&c->dst, BDADDR_ANY); > > + if ((src_match && dst_any) || (src_any && > dst_match) || > > + (src_any && dst_any)) > > + c1 = c; > > + } > > } > > } > > >
On Tue, 30 Aug 2022 01:21:58 +0530 Elliott, Robert (Servers) wrote: > > -----Original Message----- > > From: Siddh Raman Pant code@siddh.me> > > Sent: Thursday, August 25, 2022 6:01 AM > > To: palmer@rivosinc.com > > Cc: davem@davemloft.net; edumazet@google.com; johan.hedberg@gmail.com; > > kuba@kernel.org; linux-bluetooth@vger.kernel.org; linux- > > kernel@vger.kernel.org; linux@rivosinc.com; luiz.dentz@gmail.com; > > marcel@holtmann.org; netdev@vger.kernel.org; pabeni@redhat.com > > Subject: Re: [PATCH] Bluetooth: L2CAP: Elide a string overflow warning > > > > On Fri, 12 Aug 2022 11:22:49 +0530 Palmer Dabbelt wrote: > > > From: Palmer Dabbelt palmer@rivosinc.com> > > > > > > Without this I get a string op warning related to copying from a > > > possibly NULL pointer. I think the warning is spurious, but it's > > > tripping up allmodconfig. > > > > I think it is not spurious, and is due to the following commit: > > d0be8347c623 ("Bluetooth: L2CAP: Fix use-after-free caused by l2cap_chan_put") > > That commit was OK - it added an "if (!c) continue" to handle if > the value c is changed to NULL. > > > The following commit fixes a similar problem (added the NULL check on line > > 1996): > > 332f1795ca20 ("Bluetooth: L2CAP: Fix l2cap_global_chan_by_psm regression") > > That commit wiped out the "if (!c) continue" path escape clause > from the previous patch, introducing a path back to code that > doesn't check for NULL: You are correct, thanks for clarifying. Sorry for getting it reversed. So I think this patch can be modified to just introduce back the escape clause rather than having an extra indentation. Thanks, Siddh
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c index cbe0cae73434..be7f47e52119 100644 --- a/net/bluetooth/l2cap_core.c +++ b/net/bluetooth/l2cap_core.c @@ -2000,11 +2000,13 @@ static struct l2cap_chan *l2cap_global_chan_by_psm(int state, __le16 psm, } /* Closest match */ - src_any = !bacmp(&c->src, BDADDR_ANY); - dst_any = !bacmp(&c->dst, BDADDR_ANY); - if ((src_match && dst_any) || (src_any && dst_match) || - (src_any && dst_any)) - c1 = c; + if (c) { + src_any = !bacmp(&c->src, BDADDR_ANY); + dst_any = !bacmp(&c->dst, BDADDR_ANY); + if ((src_match && dst_any) || (src_any && dst_match) || + (src_any && dst_any)) + c1 = c; + } } }