Dear Edward,
Thank you very much for looking into this and sending a patch. Should
you resent, I’d make the summary about the change and not the issue. Maybe:
net/socket: Ensure length of input socket option param >= sizeof(int)
Kind regards,
Paul
The optlen value passed by syzbot to _sys_setsockopt() is 2, which results in
only 2 bytes being allocated when allocating memory to kernel_optval, and the
optval size passed when calling the function copy_from_sockptr() is 4 bytes.
Here, optlen is determined uniformly in the entry function __sys_setsockopt().
If its value is less than 4, the parameter is considered invalid.
Reported-by: [email protected]
Reported-by: [email protected]
Reported-by: [email protected]
Signed-off-by: Edward Adam Davis <[email protected]>
---
net/socket.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/socket.c b/net/socket.c
index e5f3af49a8b6..ac8fd4f6ebfe 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -2327,6 +2327,9 @@ int __sys_setsockopt(int fd, int level, int optname, char __user *user_optval,
int err, fput_needed;
struct socket *sock;
+ if (optlen < sizeof(int))
+ return -EINVAL;
+
sock = sockfd_lookup_light(fd, &err, &fput_needed);
if (!sock)
return err;
--
2.43.0
On 4/9/24 14:15, Edward Adam Davis wrote:
> The optlen value passed by syzbot to _sys_setsockopt() is 2, which results in
> only 2 bytes being allocated when allocating memory to kernel_optval, and the
> optval size passed when calling the function copy_from_sockptr() is 4 bytes.
> Here, optlen is determined uniformly in the entry function __sys_setsockopt().
> If its value is less than 4, the parameter is considered invalid.
>
> Reported-by: [email protected]
> Reported-by: [email protected]
> Reported-by: [email protected]
> Signed-off-by: Edward Adam Davis <[email protected]>
I think I gave my feedback already.
Please do not ignore maintainers feedback.
This patch is absolutely wrong.
Some setsockopt() deal with optlen == 1 just fine, thank you very much.
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=842803
---Test result---
Test Summary:
CheckPatch FAIL 0.88 seconds
GitLint FAIL 0.46 seconds
SubjectPrefix FAIL 0.43 seconds
BuildKernel PASS 31.15 seconds
CheckAllWarning PASS 33.29 seconds
CheckSparse PASS 38.65 seconds
CheckSmatch FAIL 35.17 seconds
BuildKernel32 PASS 29.25 seconds
TestRunnerSetup PASS 532.15 seconds
TestRunner_l2cap-tester FAIL 17.25 seconds
TestRunner_iso-tester FAIL 35.88 seconds
TestRunner_bnep-tester PASS 4.74 seconds
TestRunner_mgmt-tester FAIL 113.82 seconds
TestRunner_rfcomm-tester PASS 7.51 seconds
TestRunner_sco-tester FAIL 15.66 seconds
TestRunner_ioctl-tester PASS 7.73 seconds
TestRunner_mesh-tester PASS 5.73 seconds
TestRunner_smp-tester PASS 6.71 seconds
TestRunner_userchan-tester PASS 4.79 seconds
IncrementalBuild PASS 28.18 seconds
Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
net/socket: Ensure length of input socket option param >= sizeof(int)
WARNING: Prefer a maximum 75 chars per line (possible unwrapped commit description?)
#86:
The optlen value passed by syzbot to _sys_setsockopt() is 2, which results in
WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report
#92:
Reported-by: [email protected]
Reported-by: [email protected]
WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report
#93:
Reported-by: [email protected]
Reported-by: [email protected]
WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report
#94:
Reported-by: [email protected]
Signed-off-by: Edward Adam Davis <[email protected]>
total: 0 errors, 4 warnings, 0 checks, 9 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/src/13622424.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
Desc: Run gitlint
Output:
net/socket: Ensure length of input socket option param >= sizeof(int)
WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search
6: B2 Line has trailing whitespace: "Here, optlen is determined uniformly in the entry function __sys_setsockopt(). "
##############################
Test: SubjectPrefix - FAIL
Desc: Check subject contains "Bluetooth" prefix
Output:
"Bluetooth: " prefix is not specified in the subject
##############################
Test: CheckSmatch - FAIL
Desc: Run smatch tool with source
Output:
Segmentation fault (core dumped)
make[4]: *** [scripts/Makefile.build:244: net/bluetooth/hci_core.o] Error 139
make[4]: *** Deleting file 'net/bluetooth/hci_core.o'
make[3]: *** [scripts/Makefile.build:485: net/bluetooth] Error 2
make[2]: *** [scripts/Makefile.build:485: net] Error 2
make[2]: *** Waiting for unfinished jobs....
Segmentation fault (core dumped)
make[4]: *** [scripts/Makefile.build:244: drivers/bluetooth/bcm203x.o] Error 139
make[4]: *** Deleting file 'drivers/bluetooth/bcm203x.o'
make[4]: *** Waiting for unfinished jobs....
make[3]: *** [scripts/Makefile.build:485: drivers/bluetooth] Error 2
make[2]: *** [scripts/Makefile.build:485: drivers] Error 2
make[1]: *** [/github/workspace/src/src/Makefile:1919: .] Error 2
make: *** [Makefile:240: __sub-make] Error 2
##############################
Test: TestRunner_l2cap-tester - FAIL
Desc: Run l2cap-tester with test-runner
Output:
Total: 55, Passed: 40 (72.7%), Failed: 15, Not Run: 0
Failed Test Cases
L2CAP BR/EDR Client SSP - Success 2 Failed 0.072 seconds
L2CAP BR/EDR Client PIN Code - Success Failed 0.057 seconds
L2CAP LE Client SMP - Success Failed 0.060 seconds
L2CAP Ext-Flowctl Client - Success Failed 0.056 seconds
L2CAP Ext-Flowctl Client - Close Failed 0.064 seconds
L2CAP Ext-Flowctl Client - Timeout Failed 0.061 seconds
L2CAP Ext-Flowctl Client, Direct Advertising - Success Failed 0.063 seconds
L2CAP Ext-Flowctl Client SMP - Success Failed 0.069 seconds
L2CAP Ext-Flowctl Client - Command Reject Failed 0.064 seconds
L2CAP Ext-Flowctl Client - Open two sockets Failed 0.061 seconds
L2CAP Ext-Flowctl Client - Open two sockets close one Failed 0.066 seconds
L2CAP LE ATT Client - Success Failed 0.063 seconds
L2CAP LE EATT Client - Success Failed 0.064 seconds
L2CAP LE EATT Server - Success Failed 0.060 seconds
L2CAP LE EATT Server - Reject Failed 0.059 seconds
##############################
Test: TestRunner_iso-tester - FAIL
Desc: Run iso-tester with test-runner
Output:
Total: 121, Passed: 120 (99.2%), Failed: 1, Not Run: 0
Failed Test Cases
ISO Connect Suspend - Success Failed 4.164 seconds
##############################
Test: TestRunner_mgmt-tester - FAIL
Desc: Run mgmt-tester with test-runner
Output:
Total: 492, Passed: 489 (99.4%), Failed: 1, Not Run: 2
Failed Test Cases
LL Privacy - Add Device 5 (2 Devices to RL) Failed 0.171 seconds
##############################
Test: TestRunner_sco-tester - FAIL
Desc: Run sco-tester with test-runner
Output:
Total: 15, Passed: 12 (80.0%), Failed: 3, Not Run: 0
Failed Test Cases
Basic SCO Set Socket Option - Success Failed 0.085 seconds
eSCO mSBC - Success Failed 0.078 seconds
SCO mSBC 1.1 - Failure Failed 0.079 seconds
---
Regards,
Linux Bluetooth
If optlen < sizeof(u32) it will trigger oob, so take the min of them.
Reported-by: [email protected]
Signed-off-by: Edward Adam Davis <[email protected]>
---
net/bluetooth/rfcomm/sock.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
index b54e8a530f55..42c55c756b51 100644
--- a/net/bluetooth/rfcomm/sock.c
+++ b/net/bluetooth/rfcomm/sock.c
@@ -629,7 +629,7 @@ static int rfcomm_sock_setsockopt_old(struct socket *sock, int optname,
switch (optname) {
case RFCOMM_LM:
- if (copy_from_sockptr(&opt, optval, sizeof(u32))) {
+ if (copy_from_sockptr(&opt, optval, min_t(int, sizeof(u32), optlen))) {
err = -EFAULT;
break;
}
--
2.43.0
On Tue, 9 Apr 2024 15:07:41 +0200, Eric Dumazet wrote:
> > The optlen value passed by syzbot to _sys_setsockopt() is 2, which results in
> > only 2 bytes being allocated when allocating memory to kernel_optval, and the
> > optval size passed when calling the function copy_from_sockptr() is 4 bytes.
> > Here, optlen is determined uniformly in the entry function __sys_setsockopt().
> > If its value is less than 4, the parameter is considered invalid.
> >
> > Reported-by: [email protected]
> > Reported-by: [email protected]
> > Reported-by: [email protected]
> > Signed-off-by: Edward Adam Davis <[email protected]>
>
>
> I think I gave my feedback already.
>
> Please do not ignore maintainers feedback.
>
> This patch is absolutely wrong.
>
> Some setsockopt() deal with optlen == 1 just fine, thank you very much.
It's better to use evidence to support your claim, rather than your "maintainer" title.
Hi Edward,
On Tue, Apr 9, 2024 at 9:36 AM Edward Adam Davis <[email protected]> wrote:
>
> If optlen < sizeof(u32) it will trigger oob, so take the min of them.
>
> Reported-by: [email protected]
> Signed-off-by: Edward Adam Davis <[email protected]>
> ---
> net/bluetooth/rfcomm/sock.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
> index b54e8a530f55..42c55c756b51 100644
> --- a/net/bluetooth/rfcomm/sock.c
> +++ b/net/bluetooth/rfcomm/sock.c
> @@ -629,7 +629,7 @@ static int rfcomm_sock_setsockopt_old(struct socket *sock, int optname,
>
> switch (optname) {
> case RFCOMM_LM:
> - if (copy_from_sockptr(&opt, optval, sizeof(u32))) {
> + if (copy_from_sockptr(&opt, optval, min_t(int, sizeof(u32), optlen))) {
> err = -EFAULT;
> break;
> }
> --
> 2.43.0
This has been dealt with already:
https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git/commit/?id=ee77912bc0bbd78fceb785a81cc9108fa954982f
--
Luiz Augusto von Dentz
This is an automated email and please do not reply to this email.
Dear Submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
While preparing the CI tests, the patches you submitted couldn't be applied to the current HEAD of the repository.
----- Output -----
error: patch failed: net/bluetooth/rfcomm/sock.c:629
error: net/bluetooth/rfcomm/sock.c: patch does not apply
hint: Use 'git am --show-current-patch' to see the failed patch
Please resolve the issue and submit the patches again.
---
Regards,
Linux Bluetooth