* [PATCH] can: fix warning in bcm_connect/proc_register
@ 2016-10-24 19:11 Oliver Hartkopp
2016-10-24 20:10 ` Cong Wang
2016-10-27 8:47 ` Marc Kleine-Budde
0 siblings, 2 replies; 8+ messages in thread
From: Oliver Hartkopp @ 2016-10-24 19:11 UTC (permalink / raw)
To: andreyknvl, xiyou.wangcong, davem
Cc: linux-can, netdev, linux-kernel, syzkaller, kcc, glider, dvyukov,
edumazet, Oliver Hartkopp
Andrey Konovalov reported an issue with proc_register in bcm.c.
As suggested by Cong Wang this patch adds a lock_sock() protection and
a check for unsuccessful proc_create_data() in bcm_connect().
Reference: http://marc.info/?l=linux-netdev&m=147732648731237
Reported-by: Andrey Konovalov <andreyknvl@google.com>
Suggested-by: Cong Wang <xiyou.wangcong@gmail.com>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
net/can/bcm.c | 32 +++++++++++++++++++++++---------
1 file changed, 23 insertions(+), 9 deletions(-)
diff --git a/net/can/bcm.c b/net/can/bcm.c
index 8e999ff..8af9d25 100644
--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -1549,24 +1549,31 @@ static int bcm_connect(struct socket *sock, struct sockaddr *uaddr, int len,
struct sockaddr_can *addr = (struct sockaddr_can *)uaddr;
struct sock *sk = sock->sk;
struct bcm_sock *bo = bcm_sk(sk);
+ int ret = 0;
if (len < sizeof(*addr))
return -EINVAL;
- if (bo->bound)
- return -EISCONN;
+ lock_sock(sk);
+
+ if (bo->bound) {
+ ret = -EISCONN;
+ goto fail;
+ }
/* bind a device to this socket */
if (addr->can_ifindex) {
struct net_device *dev;
dev = dev_get_by_index(&init_net, addr->can_ifindex);
- if (!dev)
- return -ENODEV;
-
+ if (!dev) {
+ ret = -ENODEV;
+ goto fail;
+ }
if (dev->type != ARPHRD_CAN) {
dev_put(dev);
- return -ENODEV;
+ ret = -ENODEV;
+ goto fail;
}
bo->ifindex = dev->ifindex;
@@ -1577,17 +1584,24 @@ static int bcm_connect(struct socket *sock, struct sockaddr *uaddr, int len,
bo->ifindex = 0;
}
- bo->bound = 1;
-
if (proc_dir) {
/* unique socket address as filename */
sprintf(bo->procname, "%lu", sock_i_ino(sk));
bo->bcm_proc_read = proc_create_data(bo->procname, 0644,
proc_dir,
&bcm_proc_fops, sk);
+ if (!bo->bcm_proc_read) {
+ ret = -ENOMEM;
+ goto fail;
+ }
}
- return 0;
+ bo->bound = 1;
+
+fail:
+ release_sock(sk);
+
+ return ret;
}
static int bcm_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
--
2.9.3
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] can: fix warning in bcm_connect/proc_register
2016-10-24 19:11 [PATCH] can: fix warning in bcm_connect/proc_register Oliver Hartkopp
@ 2016-10-24 20:10 ` Cong Wang
2016-10-24 20:17 ` Cong Wang
2016-10-27 8:47 ` Marc Kleine-Budde
1 sibling, 1 reply; 8+ messages in thread
From: Cong Wang @ 2016-10-24 20:10 UTC (permalink / raw)
To: Oliver Hartkopp
Cc: Andrey Konovalov, David Miller, linux-can,
Linux Kernel Network Developers, LKML, syzkaller,
Kostya Serebryany, Alexander Potapenko, Dmitry Vyukov,
Eric Dumazet
On Mon, Oct 24, 2016 at 12:11 PM, Oliver Hartkopp
<socketcan@hartkopp.net> wrote:
> if (proc_dir) {
> /* unique socket address as filename */
> sprintf(bo->procname, "%lu", sock_i_ino(sk));
> bo->bcm_proc_read = proc_create_data(bo->procname, 0644,
> proc_dir,
> &bcm_proc_fops, sk);
> + if (!bo->bcm_proc_read) {
> + ret = -ENOMEM;
> + goto fail;
> + }
Well, I meant we need to call proc_create_data() once per socket,
so we need a check before proc_create_data() too.
Thanks.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] can: fix warning in bcm_connect/proc_register
2016-10-24 20:10 ` Cong Wang
@ 2016-10-24 20:17 ` Cong Wang
2016-10-25 12:12 ` Andrey Konovalov
2016-10-27 8:45 ` Marc Kleine-Budde
0 siblings, 2 replies; 8+ messages in thread
From: Cong Wang @ 2016-10-24 20:17 UTC (permalink / raw)
To: Oliver Hartkopp
Cc: Andrey Konovalov, David Miller, linux-can,
Linux Kernel Network Developers, LKML, syzkaller,
Kostya Serebryany, Alexander Potapenko, Dmitry Vyukov,
Eric Dumazet
On Mon, Oct 24, 2016 at 1:10 PM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Mon, Oct 24, 2016 at 12:11 PM, Oliver Hartkopp
> <socketcan@hartkopp.net> wrote:
>> if (proc_dir) {
>> /* unique socket address as filename */
>> sprintf(bo->procname, "%lu", sock_i_ino(sk));
>> bo->bcm_proc_read = proc_create_data(bo->procname, 0644,
>> proc_dir,
>> &bcm_proc_fops, sk);
>> + if (!bo->bcm_proc_read) {
>> + ret = -ENOMEM;
>> + goto fail;
>> + }
>
> Well, I meant we need to call proc_create_data() once per socket,
> so we need a check before proc_create_data() too.
Hmm, bo->bound should guarantee it, so never mind, your patch
looks fine.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] can: fix warning in bcm_connect/proc_register
2016-10-24 20:17 ` Cong Wang
@ 2016-10-25 12:12 ` Andrey Konovalov
2016-10-27 8:45 ` Marc Kleine-Budde
1 sibling, 0 replies; 8+ messages in thread
From: Andrey Konovalov @ 2016-10-25 12:12 UTC (permalink / raw)
To: Cong Wang
Cc: Oliver Hartkopp, David Miller, linux-can,
Linux Kernel Network Developers, LKML, syzkaller,
Kostya Serebryany, Alexander Potapenko, Dmitry Vyukov,
Eric Dumazet
Hi Oliver,
I can confirm that your patch fixes the warnings for me.
Tested-by: Andrey Konovalov <andreyknvl@google.com>
On Mon, Oct 24, 2016 at 10:17 PM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Mon, Oct 24, 2016 at 1:10 PM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>> On Mon, Oct 24, 2016 at 12:11 PM, Oliver Hartkopp
>> <socketcan@hartkopp.net> wrote:
>>> if (proc_dir) {
>>> /* unique socket address as filename */
>>> sprintf(bo->procname, "%lu", sock_i_ino(sk));
>>> bo->bcm_proc_read = proc_create_data(bo->procname, 0644,
>>> proc_dir,
>>> &bcm_proc_fops, sk);
>>> + if (!bo->bcm_proc_read) {
>>> + ret = -ENOMEM;
>>> + goto fail;
>>> + }
>>
>> Well, I meant we need to call proc_create_data() once per socket,
>> so we need a check before proc_create_data() too.
>
> Hmm, bo->bound should guarantee it, so never mind, your patch
> looks fine.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] can: fix warning in bcm_connect/proc_register
2016-10-24 20:17 ` Cong Wang
2016-10-25 12:12 ` Andrey Konovalov
@ 2016-10-27 8:45 ` Marc Kleine-Budde
2016-10-27 16:28 ` Cong Wang
1 sibling, 1 reply; 8+ messages in thread
From: Marc Kleine-Budde @ 2016-10-27 8:45 UTC (permalink / raw)
To: Cong Wang, Oliver Hartkopp
Cc: Andrey Konovalov, David Miller, linux-can,
Linux Kernel Network Developers, LKML, syzkaller,
Kostya Serebryany, Alexander Potapenko, Dmitry Vyukov,
Eric Dumazet
[-- Attachment #1.1: Type: text/plain, Size: 1273 bytes --]
On 10/24/2016 10:17 PM, Cong Wang wrote:
> On Mon, Oct 24, 2016 at 1:10 PM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>> On Mon, Oct 24, 2016 at 12:11 PM, Oliver Hartkopp
>> <socketcan@hartkopp.net> wrote:
>>> if (proc_dir) {
>>> /* unique socket address as filename */
>>> sprintf(bo->procname, "%lu", sock_i_ino(sk));
>>> bo->bcm_proc_read = proc_create_data(bo->procname, 0644,
>>> proc_dir,
>>> &bcm_proc_fops, sk);
>>> + if (!bo->bcm_proc_read) {
>>> + ret = -ENOMEM;
>>> + goto fail;
>>> + }
>>
>> Well, I meant we need to call proc_create_data() once per socket,
>> so we need a check before proc_create_data() too.
>
> Hmm, bo->bound should guarantee it, so never mind, your patch
> looks fine.
Can I add your Acked-by?
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] can: fix warning in bcm_connect/proc_register
2016-10-24 19:11 [PATCH] can: fix warning in bcm_connect/proc_register Oliver Hartkopp
2016-10-24 20:10 ` Cong Wang
@ 2016-10-27 8:47 ` Marc Kleine-Budde
1 sibling, 0 replies; 8+ messages in thread
From: Marc Kleine-Budde @ 2016-10-27 8:47 UTC (permalink / raw)
To: Oliver Hartkopp, andreyknvl, xiyou.wangcong, davem
Cc: linux-can, netdev, linux-kernel, syzkaller, kcc, glider, dvyukov,
edumazet
[-- Attachment #1.1: Type: text/plain, Size: 856 bytes --]
On 10/24/2016 09:11 PM, Oliver Hartkopp wrote:
> Andrey Konovalov reported an issue with proc_register in bcm.c.
> As suggested by Cong Wang this patch adds a lock_sock() protection and
> a check for unsuccessful proc_create_data() in bcm_connect().
>
> Reference: http://marc.info/?l=linux-netdev&m=147732648731237
>
> Reported-by: Andrey Konovalov <andreyknvl@google.com>
> Suggested-by: Cong Wang <xiyou.wangcong@gmail.com>
> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Added with Andrey Konovalov Tested-by to can/master with stable on Cc.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] can: fix warning in bcm_connect/proc_register
2016-10-27 8:45 ` Marc Kleine-Budde
@ 2016-10-27 16:28 ` Cong Wang
2016-10-28 8:58 ` Marc Kleine-Budde
0 siblings, 1 reply; 8+ messages in thread
From: Cong Wang @ 2016-10-27 16:28 UTC (permalink / raw)
To: Marc Kleine-Budde
Cc: Oliver Hartkopp, Andrey Konovalov, David Miller, linux-can,
Linux Kernel Network Developers, LKML, syzkaller,
Kostya Serebryany, Alexander Potapenko, Dmitry Vyukov,
Eric Dumazet
On Thu, Oct 27, 2016 at 1:45 AM, Marc Kleine-Budde <mkl@pengutronix.de> wrote:
> On 10/24/2016 10:17 PM, Cong Wang wrote:
>> On Mon, Oct 24, 2016 at 1:10 PM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>>> On Mon, Oct 24, 2016 at 12:11 PM, Oliver Hartkopp
>>> <socketcan@hartkopp.net> wrote:
>>>> if (proc_dir) {
>>>> /* unique socket address as filename */
>>>> sprintf(bo->procname, "%lu", sock_i_ino(sk));
>>>> bo->bcm_proc_read = proc_create_data(bo->procname, 0644,
>>>> proc_dir,
>>>> &bcm_proc_fops, sk);
>>>> + if (!bo->bcm_proc_read) {
>>>> + ret = -ENOMEM;
>>>> + goto fail;
>>>> + }
>>>
>>> Well, I meant we need to call proc_create_data() once per socket,
>>> so we need a check before proc_create_data() too.
>>
>> Hmm, bo->bound should guarantee it, so never mind, your patch
>> looks fine.
>
> Can I add your Acked-by?
Of course.
Acked-by: Cong Wang <xiyou.wangcong@gmail.com>
Thanks.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] can: fix warning in bcm_connect/proc_register
2016-10-27 16:28 ` Cong Wang
@ 2016-10-28 8:58 ` Marc Kleine-Budde
0 siblings, 0 replies; 8+ messages in thread
From: Marc Kleine-Budde @ 2016-10-28 8:58 UTC (permalink / raw)
To: Cong Wang
Cc: Oliver Hartkopp, Andrey Konovalov, David Miller, linux-can,
Linux Kernel Network Developers, LKML, syzkaller,
Kostya Serebryany, Alexander Potapenko, Dmitry Vyukov,
Eric Dumazet
[-- Attachment #1.1: Type: text/plain, Size: 525 bytes --]
On 10/27/2016 06:28 PM, Cong Wang wrote:
>>> Hmm, bo->bound should guarantee it, so never mind, your patch
>>> looks fine.
>>
>> Can I add your Acked-by?
>
> Of course.
>
> Acked-by: Cong Wang <xiyou.wangcong@gmail.com>
Thanks,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-10-28 8:58 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-24 19:11 [PATCH] can: fix warning in bcm_connect/proc_register Oliver Hartkopp
2016-10-24 20:10 ` Cong Wang
2016-10-24 20:17 ` Cong Wang
2016-10-25 12:12 ` Andrey Konovalov
2016-10-27 8:45 ` Marc Kleine-Budde
2016-10-27 16:28 ` Cong Wang
2016-10-28 8:58 ` Marc Kleine-Budde
2016-10-27 8:47 ` Marc Kleine-Budde
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.