Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1507638 > unrolled thread
| Started by | Oliver Hartkopp <socketcan@hartkopp.net> |
|---|---|
| First post | 2016-10-24 21:20 +0200 |
| Last post | 2016-10-24 22:20 +0200 |
| Articles | 4 — 3 participants |
Back to article view | Back to linux.kernel
[PATCH] can: fix warning in bcm_connect/proc_register Oliver Hartkopp <socketcan@hartkopp.net> - 2016-10-24 21:20 +0200
Re: [PATCH] can: fix warning in bcm_connect/proc_register Cong Wang <xiyou.wangcong@gmail.com> - 2016-10-24 22:20 +0200
Re: [PATCH] can: fix warning in bcm_connect/proc_register Andrey Konovalov <andreyknvl@google.com> - 2016-10-25 14:20 +0200
Re: [PATCH] can: fix warning in bcm_connect/proc_register Cong Wang <xiyou.wangcong@gmail.com> - 2016-10-24 22:20 +0200
| From | Oliver Hartkopp <socketcan@hartkopp.net> |
|---|---|
| Date | 2016-10-24 21:20 +0200 |
| Subject | [PATCH] can: fix warning in bcm_connect/proc_register |
| Message-ID | <svSWR-3rd-21@gated-at.bofh.it> |
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
[toc] | [next] | [standalone]
| From | Cong Wang <xiyou.wangcong@gmail.com> |
|---|---|
| Date | 2016-10-24 22:20 +0200 |
| Message-ID | <svTSV-43c-1@gated-at.bofh.it> |
| In reply to | #1507638 |
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.
[toc] | [prev] | [next] | [standalone]
| From | Andrey Konovalov <andreyknvl@google.com> |
|---|---|
| Date | 2016-10-25 14:20 +0200 |
| Message-ID | <sw8RY-5yg-11@gated-at.bofh.it> |
| In reply to | #1507686 |
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.
[toc] | [prev] | [next] | [standalone]
| From | Cong Wang <xiyou.wangcong@gmail.com> |
|---|---|
| Date | 2016-10-24 22:20 +0200 |
| Message-ID | <svTSV-43c-3@gated-at.bofh.it> |
| In reply to | #1507638 |
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.
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web