Skip to contentSkip to conversation

bluetooth: handle l2cap_create_connless_pdu() errors

PATCH In review Dan Carpenter 7 participants 8 messages Apr 22, 2010 – Apr 28, 2010 2 messages not in the archive

Subsystems (5)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 bluetooth: handle l2cap_create_connless_pdu() errors

l2cap_create_connless_pdu() can sometimes return ERR_PTR(-ENOMEM) or ERR_PTR(-EFAULT).

net/bluetooth/l2cap.c +4 -1
index 99d68c3..9753b69 100644
@@ -1626,7 +1626,10 @@ static int l2cap_sock_sendmsg(struct kiocb *iocb, struct socket *sock, struct ms
 	/* Connectionless channel */
 	if (sk->sk_type = SOCK_DGRAM) {
 		skb = l2cap_create_connless_pdu(sk, msg, len);
-		err = l2cap_do_send(sk, skb);
+		if (IS_ERR(skb))
+			err = PTR_ERR(skb);
+		else
+			err = l2cap_do_send(sk, skb);
 		goto done;
 	}
 
#1 lore raw

Hi Dan,

Quoted 10 lines · Dan Carpenter

l2cap_create_connless_pdu() can sometimes return ERR_PTR(-ENOMEM) or ERR_PTR(-EFAULT).

net/bluetooth/l2cap.c +0 -0
index 99d68c3..9753b69 100644

net/bluetooth/l2cap.c static int l2cap_sock_sendmsg(struct kiocb *iocb, struct socket *sock, struct ms · line 1634

Quoted 10 lines
net/bluetooth/l2cap.c +4 -1
@@ -1626,7 +1626,10 @@ static int l2cap_sock_sendmsg(struct kiocb *iocb, struct socket *sock, struct ms
 	/* Connectionless channel */
 	if (sk->sk_type = SOCK_DGRAM) {
 		skb = l2cap_create_connless_pdu(sk, msg, len);
-		err = l2cap_do_send(sk, skb);
+		if (IS_ERR(skb))
+			err = PTR_ERR(skb);
+		else
+			err = l2cap_do_send(sk, skb);
 		goto done;
 	}

Your fix looks ok, but we have changed l2cap_do_send() to void, so you have to modify your patch.

That change is not in the bluetooth-testingtree yet, so remote add my git tree and use the branch ertm.

git://git.profusion.mobi/users/padovan/bluetooth-testing.git

Regards,

Signature · 2 lines
Gustavo F. Padovan
http://padovan.org
#2 lore raw

PATCH v2 1/1 bluetooth: handle l2cap_create_connless_pdu() errors

l2cap_create_connless_pdu() can sometimes return ERR_PTR(-ENOMEM) or ERR_PTR(-EFAULT).

---
In v2 I wrote the patch on top of Gustavo Padovon's devel tree

net/bluetooth/l2cap.c +6 -2
index a668ef4..b32682c 100644
@@ -1712,8 +1712,12 @@ static int l2cap_sock_sendmsg(struct kiocb *iocb, struct socket *sock, struct ms
 	/* Connectionless channel */
 	if (sk->sk_type = SOCK_DGRAM) {
 		skb = l2cap_create_connless_pdu(sk, msg, len);
-		l2cap_do_send(sk, skb);
-		err = len;
+		if (IS_ERR(skb)) {
+			err = PTR_ERR(skb);
+		} else {
+			l2cap_do_send(sk, skb);
+			err = len;
+		}
 		goto done;
 	}
 
#3 lore raw

Hi Dan,

Quoted 12 lines · Dan Carpenter

l2cap_create_connless_pdu() can sometimes return ERR_PTR(-ENOMEM) or ERR_PTR(-EFAULT).

---
In v2 I wrote the patch on top of Gustavo Padovon's devel tree

net/bluetooth/l2cap.c +0 -0
index a668ef4..b32682c 100644

net/bluetooth/l2cap.c static int l2cap_sock_sendmsg(struct kiocb *iocb, struct socket *sock, struct ms · line 1716

Quoted 7 lines
net/bluetooth/l2cap.c +2 -2
@@ -1712,8 +1712,12 @@ static int l2cap_sock_sendmsg(struct kiocb *iocb, struct socket *sock, struct ms
 	/* Connectionless channel */
 	if (sk->sk_type = SOCK_DGRAM) {
 		skb = l2cap_create_connless_pdu(sk, msg, len);
-		l2cap_do_send(sk, skb);
-		err = len;
+		if (IS_ERR(skb)) {
+			err = PTR_ERR(skb);

Remove the braces on the 'if', then we are fine to pick your patch ;)

net/bluetooth/l2cap.c static int l2cap_sock_sendmsg(struct kiocb *iocb, struct socket *sock, struct ms · line 1723

Quoted 7 lines
net/bluetooth/l2cap.c +4 -0
+		} else {
+			l2cap_do_send(sk, skb);
+			err = len;
+		}
 		goto done;
 	}
 

Regards,

Signature · 2 lines
Gustavo F. Padovan
http://padovan.org
maintainer
#4 lore raw

From: "Gustavo F. Padovan" <[email protected]>
Date: Mon, 26 Apr 2010 12:09:19 -0300

Quoted 10 lines

Hi Dan,

Quoted 6 lines · Dan Carpenter

l2cap_create_connless_pdu() can sometimes return ERR_PTR(-ENOMEM) or ERR_PTR(-EFAULT).

---
In v2 I wrote the patch on top of Gustavo Padovon's devel tree

This is the kind of bug that could cause a crash if the path actually executes.

Therefore it tires me that that submitter was told to regenerate this patch against some devel tree that is -next bound, when in fact this is the kind of fix that warrants inclusion right now into net-2.6

Marcel, please do whatever magic you need to so I can get this into Linus's tree as I did the rest of the ERR_PTR() fixes from Dan already. No reason to treat Bluetooth special and defer these fixes to -next.

Thanks.

A message that is not in the archive (sent off-list or not yet received): 20100426.111259.112594696.davem-fT/PcQaiUtIeIZ0/[email protected]

#6 lore raw

Hi David,

Quoted 14 lines · David Miller

From: "Gustavo F. Padovan" <[email protected]>
Date: Mon, 26 Apr 2010 12:09:19 -0300

Quoted 10 lines

Hi Dan,

Quoted 6 lines · Dan Carpenter

l2cap_create_connless_pdu() can sometimes return ERR_PTR(-ENOMEM) or ERR_PTR(-EFAULT).

---
In v2 I wrote the patch on top of Gustavo Padovon's devel tree

Quoted 6 lines

This is the kind of bug that could cause a crash if the path actually executes.

Therefore it tires me that that submitter was told to regenerate this patch against some devel tree that is -next bound, when in fact this is the kind of fix that warrants inclusion right now into net-2.6

My bad here. So I think we should pick the first version of the Dan's patch. It applies against bluetooth-testing right now and then against net-2.6 too.

Marcel, is that ok to you?

Quoted 6 lines

Marcel, please do whatever magic you need to so I can get this into Linus's tree as I did the rest of the ERR_PTR() fixes from Dan already. No reason to treat Bluetooth special and defer these fixes to -next.

Thanks.

Signature · 2 lines
Gustavo F. Padovan
http://padovan.org
#7 lore raw
In reply to a message not in the archive

Hi Dave,

Quoted 12 lines
Quoted 8 lines · Dan Carpenter
Quoted 7 lines

l2cap_create_connless_pdu() can sometimes return ERR_PTR(-ENOMEM) or ERR_PTR(-EFAULT).

---
In v2 I wrote the patch on top of Gustavo Padovon's devel tree

This is the kind of bug that could cause a crash if the path actually executes.

Quoted 7 lines

Therefore it tires me that that submitter was told to regenerate this patch against some devel tree that is -next bound, when in fact this is the kind of fix that warrants inclusion right now into net-2.6

Marcel, please do whatever magic you need to so I can get this into Linus's tree as I did the rest of the ERR_PTR() fixes from Dan already. No reason to treat Bluetooth special and defer these fixes to -next.

my bad here. I looked at the patch and thought it was only an issue inside the development tree. So was letting Gustavo sort this out. You are 100% right, this needs to be fixed right away. If you wanna take this directly, then

Regards

Marcel

A message that is not in the archive (sent off-list or not yet received): 1272333926.22838.116.camel-bi+AKbBUZKY6gyzm1THtWbp2dZbC/[email protected]

Applied
#9 lore raw

From: Marcel Holtmann <[email protected]>
Date: Tue, 27 Apr 2010 04:05:26 +0200

Quoted 6 lines

my bad here. I looked at the patch and thought it was only an issue inside the development tree. So was letting Gustavo sort this out. You are 100% right, this needs to be fixed right away. If you wanna take this directly, then

I applied the original patch with your ACK, thanks Marcel.