diff mbox series

Bluetooth: L2CAP: Fix use-after-free in l2cap_sock_ready_cb

Message ID 20230526181647.3074391-1-iam@sung-woo.kim
State Accepted
Commit 8f514cf700508b11946b55eac56bf9acd2b3a884
Headers show
Series Bluetooth: L2CAP: Fix use-after-free in l2cap_sock_ready_cb | expand

Commit Message

Sungwoo Kim May 26, 2023, 6:16 p.m. UTC
>    net/bluetooth/l2cap_sock.c: In function 'l2cap_sock_release':
> >> net/bluetooth/l2cap_sock.c:1418:9: error: implicit declaration of function 'l2cap_sock_cleanup_listen'; did you mean 'l2cap_sock_listen'? [-Werror=implicit-function-declaration]

Fix this error

>     1418 |         l2cap_sock_cleanup_listen(sk);
>          |         ^~~~~~~~~~~~~~~~~~~~~~~~~
>          |         l2cap_sock_listen
>    net/bluetooth/l2cap_sock.c: At top level:
> >> net/bluetooth/l2cap_sock.c:1436:13: warning: conflicting types for 'l2cap_sock_cleanup_listen'; have 'void(struct sock *)'
>     1436 | static void l2cap_sock_cleanup_listen(struct sock *parent)
>          |             ^~~~~~~~~~~~~~~~~~~~~~~~~
> >> net/bluetooth/l2cap_sock.c:1436:13: error: static declaration of 'l2cap_sock_cleanup_listen' follows non-static declaration
>    net/bluetooth/l2cap_sock.c:1418:9: note: previous implicit declaration of 'l2cap_sock_cleanup_listen' with type 'void(struct sock *)'
>     1418 |         l2cap_sock_cleanup_listen(sk);
>          |         ^~~~~~~~~~~~~~~~~~~~~~~~~
>    cc1: some warnings being treated as errors

Signed-off-by: Sungwoo Kim <iam@sung-woo.kim>
---
 net/bluetooth/l2cap_sock.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

bluez.test.bot@gmail.com May 26, 2023, 6:57 p.m. UTC | #1
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=751481

---Test result---

Test Summary:
CheckPatch                    FAIL      0.81 seconds
GitLint                       FAIL      0.54 seconds
SubjectPrefix                 PASS      0.09 seconds
BuildKernel                   PASS      31.67 seconds
CheckAllWarning               PASS      34.90 seconds
CheckSparse                   PASS      39.38 seconds
CheckSmatch                   PASS      110.87 seconds
BuildKernel32                 PASS      30.74 seconds
TestRunnerSetup               PASS      441.52 seconds
TestRunner_l2cap-tester       PASS      16.65 seconds
TestRunner_iso-tester         PASS      21.28 seconds
TestRunner_bnep-tester        PASS      5.47 seconds
TestRunner_mgmt-tester        PASS      110.94 seconds
TestRunner_rfcomm-tester      PASS      8.76 seconds
TestRunner_sco-tester         PASS      8.05 seconds
TestRunner_ioctl-tester       PASS      9.27 seconds
TestRunner_mesh-tester        PASS      6.87 seconds
TestRunner_smp-tester         PASS      7.98 seconds
TestRunner_userchan-tester    PASS      5.72 seconds
IncrementalBuild              PASS      29.11 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
Bluetooth: L2CAP: Fix use-after-free in l2cap_sock_ready_cb
WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#74: 
> >> net/bluetooth/l2cap_sock.c:1418:9: error: implicit declaration of function 'l2cap_sock_cleanup_listen'; did you mean 'l2cap_sock_listen'? [-Werror=implicit-function-declaration]

ERROR: trailing whitespace
#113: FILE: net/bluetooth/l2cap_sock.c:1418:
+^I^I$

total: 1 errors, 1 warnings, 0 checks, 16 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.

NOTE: Whitespace errors detected.
      You may wish to use scripts/cleanpatch or scripts/cleanfile

/github/workspace/src/src/13257259.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:
Bluetooth: L2CAP: Fix use-after-free in l2cap_sock_ready_cb

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
4: B1 Line exceeds max length (182>80): "> >> net/bluetooth/l2cap_sock.c:1418:9: error: implicit declaration of function 'l2cap_sock_cleanup_listen'; did you mean 'l2cap_sock_listen'? [-Werror=implicit-function-declaration]"
12: B1 Line exceeds max length (127>80): "> >> net/bluetooth/l2cap_sock.c:1436:13: warning: conflicting types for 'l2cap_sock_cleanup_listen'; have 'void(struct sock *)'"
15: B1 Line exceeds max length (128>80): "> >> net/bluetooth/l2cap_sock.c:1436:13: error: static declaration of 'l2cap_sock_cleanup_listen' follows non-static declaration"
16: B1 Line exceeds max length (138>80): ">    net/bluetooth/l2cap_sock.c:1418:9: note: previous implicit declaration of 'l2cap_sock_cleanup_listen' with type 'void(struct sock *)'"


---
Regards,
Linux Bluetooth
Simon Horman May 27, 2023, 12:59 p.m. UTC | #2
On Fri, May 26, 2023 at 02:16:48PM -0400, Sungwoo Kim wrote:
> >    net/bluetooth/l2cap_sock.c: In function 'l2cap_sock_release':
> > >> net/bluetooth/l2cap_sock.c:1418:9: error: implicit declaration of function 'l2cap_sock_cleanup_listen'; did you mean 'l2cap_sock_listen'? [-Werror=implicit-function-declaration]
> 
> Fix this error
> 
> >     1418 |         l2cap_sock_cleanup_listen(sk);
> >          |         ^~~~~~~~~~~~~~~~~~~~~~~~~
> >          |         l2cap_sock_listen
> >    net/bluetooth/l2cap_sock.c: At top level:
> > >> net/bluetooth/l2cap_sock.c:1436:13: warning: conflicting types for 'l2cap_sock_cleanup_listen'; have 'void(struct sock *)'
> >     1436 | static void l2cap_sock_cleanup_listen(struct sock *parent)
> >          |             ^~~~~~~~~~~~~~~~~~~~~~~~~
> > >> net/bluetooth/l2cap_sock.c:1436:13: error: static declaration of 'l2cap_sock_cleanup_listen' follows non-static declaration
> >    net/bluetooth/l2cap_sock.c:1418:9: note: previous implicit declaration of 'l2cap_sock_cleanup_listen' with type 'void(struct sock *)'
> >     1418 |         l2cap_sock_cleanup_listen(sk);
> >          |         ^~~~~~~~~~~~~~~~~~~~~~~~~
> >    cc1: some warnings being treated as errors
> 
> Signed-off-by: Sungwoo Kim <iam@sung-woo.kim>

Hi,

I am confused about why this error occurs.
In bluetooth-next [1] I see that l2cap_sock_cleanup_listen() is defined
on line  1435 of l2cap_sock.c. And then used on line 1574.
So there should be no need for a forward declaration.

[1] a088d769ef3a ("Bluetooth: L2CAP: Fix use-after-free")

> ---
>  net/bluetooth/l2cap_sock.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> index eebe25610..3818e11a8 100644
> --- a/net/bluetooth/l2cap_sock.c
> +++ b/net/bluetooth/l2cap_sock.c
> @@ -46,6 +46,7 @@ static const struct proto_ops l2cap_sock_ops;
>  static void l2cap_sock_init(struct sock *sk, struct sock *parent);
>  static struct sock *l2cap_sock_alloc(struct net *net, struct socket *sock,
>  				     int proto, gfp_t prio, int kern);
> +static void l2cap_sock_cleanup_listen(struct sock *parent);
>  
>  bool l2cap_is_socket(struct socket *sock)
>  {
> @@ -1414,7 +1415,8 @@ static int l2cap_sock_release(struct socket *sock)
>  
>  	if (!sk)
>  		return 0;
> -
> +		

nit: The white-space on the line above was correct (no white-space)
     Now there are trailing tabs.

> +	l2cap_sock_cleanup_listen(sk);

This change may match the patch subject
but seems unrelated to the patch description.

>  	bt_sock_unlink(&l2cap_sk_list, sk);
>  
>  	err = l2cap_sock_shutdown(sock, SHUT_RDWR);
> -- 
> 2.34.1
> 
>
diff mbox series

Patch

diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index eebe25610..3818e11a8 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -46,6 +46,7 @@  static const struct proto_ops l2cap_sock_ops;
 static void l2cap_sock_init(struct sock *sk, struct sock *parent);
 static struct sock *l2cap_sock_alloc(struct net *net, struct socket *sock,
 				     int proto, gfp_t prio, int kern);
+static void l2cap_sock_cleanup_listen(struct sock *parent);
 
 bool l2cap_is_socket(struct socket *sock)
 {
@@ -1414,7 +1415,8 @@  static int l2cap_sock_release(struct socket *sock)
 
 	if (!sk)
 		return 0;
-
+		
+	l2cap_sock_cleanup_listen(sk);
 	bt_sock_unlink(&l2cap_sk_list, sk);
 
 	err = l2cap_sock_shutdown(sock, SHUT_RDWR);