* [PATCH] cmtp: fix argument error
@ 2023-03-31 6:45 Chenyuan Mi
2023-03-31 7:36 ` bluez.test.bot
2023-03-31 17:33 ` [PATCH] " Simon Horman
0 siblings, 2 replies; 5+ messages in thread
From: Chenyuan Mi @ 2023-03-31 6:45 UTC (permalink / raw)
To: isdn
Cc: marcel, johan.hedberg, luiz.dentz, davem, edumazet, kuba, pabeni,
netdev, linux-bluetooth, linux-kernel
Fix this issue by using BTPROTO_CMTP as argument instead of BTPROTO_HIDP.
Signed-off-by: Chenyuan Mi <michenyuan@huawei.com>
---
net/bluetooth/cmtp/sock.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/bluetooth/cmtp/sock.c b/net/bluetooth/cmtp/sock.c
index 96d49d9fae96..cf4370055ce2 100644
--- a/net/bluetooth/cmtp/sock.c
+++ b/net/bluetooth/cmtp/sock.c
@@ -250,7 +250,7 @@ int cmtp_init_sockets(void)
err = bt_procfs_init(&init_net, "cmtp", &cmtp_sk_list, NULL);
if (err < 0) {
BT_ERR("Failed to create CMTP proc file");
- bt_sock_unregister(BTPROTO_HIDP);
+ bt_sock_unregister(BTPROTO_CMTP);
goto error;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* RE: cmtp: fix argument error
2023-03-31 6:45 [PATCH] cmtp: fix argument error Chenyuan Mi
@ 2023-03-31 7:36 ` bluez.test.bot
2023-03-31 17:33 ` [PATCH] " Simon Horman
1 sibling, 0 replies; 5+ messages in thread
From: bluez.test.bot @ 2023-03-31 7:36 UTC (permalink / raw)
To: linux-bluetooth, michenyuan
[-- Attachment #1: Type: text/plain, Size: 1597 bytes --]
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=735678
---Test result---
Test Summary:
CheckPatch PASS 0.75 seconds
GitLint PASS 0.36 seconds
SubjectPrefix FAIL 0.41 seconds
BuildKernel PASS 39.31 seconds
CheckAllWarning PASS 43.12 seconds
CheckSparse PASS 48.21 seconds
CheckSmatch PASS 129.98 seconds
BuildKernel32 PASS 38.30 seconds
TestRunnerSetup PASS 534.77 seconds
TestRunner_l2cap-tester PASS 18.44 seconds
TestRunner_iso-tester PASS 18.96 seconds
TestRunner_bnep-tester PASS 6.32 seconds
TestRunner_mgmt-tester PASS 123.65 seconds
TestRunner_rfcomm-tester PASS 10.02 seconds
TestRunner_sco-tester PASS 8.95 seconds
TestRunner_ioctl-tester PASS 10.65 seconds
TestRunner_mesh-tester PASS 8.16 seconds
TestRunner_smp-tester PASS 9.17 seconds
TestRunner_userchan-tester PASS 6.59 seconds
IncrementalBuild PASS 35.97 seconds
Details
##############################
Test: SubjectPrefix - FAIL
Desc: Check subject contains "Bluetooth" prefix
Output:
"Bluetooth: " prefix is not specified in the subject
---
Regards,
Linux Bluetooth
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] cmtp: fix argument error
2023-03-31 6:45 [PATCH] cmtp: fix argument error Chenyuan Mi
2023-03-31 7:36 ` bluez.test.bot
@ 2023-03-31 17:33 ` Simon Horman
1 sibling, 0 replies; 5+ messages in thread
From: Simon Horman @ 2023-03-31 17:33 UTC (permalink / raw)
To: Chenyuan Mi
Cc: isdn, marcel, johan.hedberg, luiz.dentz, davem, edumazet, kuba,
pabeni, netdev, linux-bluetooth, linux-kernel
On Fri, Mar 31, 2023 at 02:45:20PM +0800, Chenyuan Mi wrote:
> Fix this issue by using BTPROTO_CMTP as argument instead of BTPROTO_HIDP.
Thanks for your patch. Some things you may want to consider:
* I think it would be good to describe what the effect of this problem is,
if it can be observed. And if not, say so. I think it would
also be useful to state how the problem was found. F.e. using a tool, or
by inspection.
* As this is described as a fix, it should probably have a fixes tag.
I think it would be:
Fixes: 8c8de589cedd ("Bluetooth: Added /proc/net/cmtp via bt_procfs_init()")
> Signed-off-by: Chenyuan Mi <michenyuan@huawei.com>
> ---
> net/bluetooth/cmtp/sock.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Code change looks good.
> diff --git a/net/bluetooth/cmtp/sock.c b/net/bluetooth/cmtp/sock.c
> index 96d49d9fae96..cf4370055ce2 100644
> --- a/net/bluetooth/cmtp/sock.c
> +++ b/net/bluetooth/cmtp/sock.c
> @@ -250,7 +250,7 @@ int cmtp_init_sockets(void)
> err = bt_procfs_init(&init_net, "cmtp", &cmtp_sk_list, NULL);
> if (err < 0) {
> BT_ERR("Failed to create CMTP proc file");
> - bt_sock_unregister(BTPROTO_HIDP);
> + bt_sock_unregister(BTPROTO_CMTP);
> goto error;
> }
>
> --
> 2.25.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] cmtp: fix argument error
2023-04-03 2:35 michenyuan
@ 2023-04-03 9:56 ` Simon Horman
0 siblings, 0 replies; 5+ messages in thread
From: Simon Horman @ 2023-04-03 9:56 UTC (permalink / raw)
To: michenyuan
Cc: isdn, marcel, johan.hedberg, luiz.dentz, davem, edumazet, kuba,
pabeni, netdev, linux-bluetooth, linux-kernel
On Mon, Apr 03, 2023 at 02:35:21AM +0000, michenyuan wrote:
> Thank you for your suggestion.
>
> This bug may not cause serious security problem. Function 'bt_sock_unregister' takes its parameter as an index and nulls the corresponding element of 'bt_proto' which is an array of pointers. When 'bt_proto' dereferences each element, it would check whether the element is empty or not. Therefore, the problem of null pointer deference does not occur.
>
> This bug is observed by manually code review.
Thanks, could I suggest that you post a v2 that looks a bit like this:
Subject: Re: [PATCH v2 net-next] bluetooth: unregister correct BTPROTO for CMTP
On error unregister BTPROTO_CMTP to match the registration earlier
in the same code-path. Without this change BTPROTO_HIDP is incorrectly
unregistered.
This bug does not appear to cause serious security problem.
The function 'bt_sock_unregister' takes its parameter as an index and NULLs
the corresponding element of 'bt_proto' which is an array of pointers. When
'bt_proto' dereferences each element, it would check whether the element is
empty or not. Therefore, the problem of null pointer deference does not
occur.
Found by inspection.
Fixes: 8c8de589cedd ("Bluetooth: Added /proc/net/cmtp via bt_procfs_init()")
Signed-off-by: ...
...
> > ---
> > net/bluetooth/cmtp/sock.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
>
> Code change looks good.
>
> > diff --git a/net/bluetooth/cmtp/sock.c b/net/bluetooth/cmtp/sock.c
> > index 96d49d9fae96..cf4370055ce2 100644
> > --- a/net/bluetooth/cmtp/sock.c
> > +++ b/net/bluetooth/cmtp/sock.c
> > @@ -250,7 +250,7 @@ int cmtp_init_sockets(void)
> > err = bt_procfs_init(&init_net, "cmtp", &cmtp_sk_list, NULL);
> > if (err < 0) {
> > BT_ERR("Failed to create CMTP proc file");
> > - bt_sock_unregister(BTPROTO_HIDP);
> > + bt_sock_unregister(BTPROTO_CMTP);
> > goto error;
> > }
> >
> > --
> > 2.25.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] cmtp: fix argument error
@ 2023-04-03 2:35 michenyuan
2023-04-03 9:56 ` Simon Horman
0 siblings, 1 reply; 5+ messages in thread
From: michenyuan @ 2023-04-03 2:35 UTC (permalink / raw)
To: Simon Horman
Cc: isdn, marcel, johan.hedberg, luiz.dentz, davem, edumazet, kuba,
pabeni, netdev, linux-bluetooth, linux-kernel
Thank you for your suggestion.
This bug may not cause serious security problem. Function 'bt_sock_unregister' takes its parameter as an index and nulls the corresponding element of 'bt_proto' which is an array of pointers. When 'bt_proto' dereferences each element, it would check whether the element is empty or not. Therefore, the problem of null pointer deference does not occur.
This bug is observed by manually code review.
----------
On Fri, Mar 31, 2023 at 02:45:20PM +0800, Chenyuan Mi wrote:
> Fix this issue by using BTPROTO_CMTP as argument instead of BTPROTO_HIDP.
Thanks for your patch. Some things you may want to consider:
* I think it would be good to describe what the effect of this problem is,
if it can be observed. And if not, say so. I think it would
also be useful to state how the problem was found. F.e. using a tool, or
by inspection.
* As this is described as a fix, it should probably have a fixes tag.
I think it would be:
Fixes: 8c8de589cedd ("Bluetooth: Added /proc/net/cmtp via bt_procfs_init()")
> Signed-off-by: Chenyuan Mi <michenyuan@huawei.com>
> ---
> net/bluetooth/cmtp/sock.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Code change looks good.
> diff --git a/net/bluetooth/cmtp/sock.c b/net/bluetooth/cmtp/sock.c
> index 96d49d9fae96..cf4370055ce2 100644
> --- a/net/bluetooth/cmtp/sock.c
> +++ b/net/bluetooth/cmtp/sock.c
> @@ -250,7 +250,7 @@ int cmtp_init_sockets(void)
> err = bt_procfs_init(&init_net, "cmtp", &cmtp_sk_list, NULL);
> if (err < 0) {
> BT_ERR("Failed to create CMTP proc file");
> - bt_sock_unregister(BTPROTO_HIDP);
> + bt_sock_unregister(BTPROTO_CMTP);
> goto error;
> }
>
> --
> 2.25.1
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-04-03 9:57 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-31 6:45 [PATCH] cmtp: fix argument error Chenyuan Mi
2023-03-31 7:36 ` bluez.test.bot
2023-03-31 17:33 ` [PATCH] " Simon Horman
2023-04-03 2:35 michenyuan
2023-04-03 9:56 ` Simon Horman
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.