* [PATCH] netlink: put module reference if dump start fails
@ 2018-02-21 3:41 Jason A. Donenfeld
2018-02-21 5:47 ` Eric Dumazet
2018-02-22 19:04 ` David Miller
0 siblings, 2 replies; 8+ messages in thread
From: Jason A. Donenfeld @ 2018-02-21 3:41 UTC (permalink / raw)
To: davem, johannes.berg, netdev, linux-kernel; +Cc: Jason A. Donenfeld
Before, if cb->start() failed, the module reference would never be put,
because cb->cb_running is intentionally false at this point. Users are
generally annoyed by this because they can no longer unload modules that
leak references. Also, it may be possible to tediously wrap a reference
counter back to zero, especially since module.c still uses atomic_inc
instead of refcount_inc.
This patch expands the error path to simply call module_put if
cb->start() fails.
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
This probably should be queued up for stable.
net/netlink/af_netlink.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 2ad445c1d27c..07e8478068f0 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -2308,7 +2308,7 @@ int __netlink_dump_start(struct sock *ssk, struct sk_buff *skb,
if (cb->start) {
ret = cb->start(cb);
if (ret)
- goto error_unlock;
+ goto error_put;
}
nlk->cb_running = true;
@@ -2328,6 +2328,8 @@ int __netlink_dump_start(struct sock *ssk, struct sk_buff *skb,
*/
return -EINTR;
+error_put:
+ module_put(control->module);
error_unlock:
sock_put(sk);
mutex_unlock(nlk->cb_mutex);
--
2.16.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] netlink: put module reference if dump start fails
2018-02-21 3:41 [PATCH] netlink: put module reference if dump start fails Jason A. Donenfeld
@ 2018-02-21 5:47 ` Eric Dumazet
2018-02-21 14:54 ` Jason A. Donenfeld
2018-02-22 19:04 ` David Miller
1 sibling, 1 reply; 8+ messages in thread
From: Eric Dumazet @ 2018-02-21 5:47 UTC (permalink / raw)
To: Jason A. Donenfeld, davem, johannes.berg, netdev, linux-kernel
On Wed, 2018-02-21 at 04:41 +0100, Jason A. Donenfeld wrote:
> Before, if cb->start() failed, the module reference would never be put,
> because cb->cb_running is intentionally false at this point. Users are
> generally annoyed by this because they can no longer unload modules that
> leak references. Also, it may be possible to tediously wrap a reference
> counter back to zero, especially since module.c still uses atomic_inc
> instead of refcount_inc.
>
> This patch expands the error path to simply call module_put if
> cb->start() fails.
>
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> ---
> This probably should be queued up for stable.
When was the bug added ? This would help a lot stable teams ...
Thanks.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] netlink: put module reference if dump start fails
2018-02-21 5:47 ` Eric Dumazet
@ 2018-02-21 14:54 ` Jason A. Donenfeld
2018-02-21 15:11 ` Eric Dumazet
0 siblings, 1 reply; 8+ messages in thread
From: Jason A. Donenfeld @ 2018-02-21 14:54 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, Berg, Johannes, Netdev, LKML
On Wed, Feb 21, 2018 at 6:47 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>> This probably should be queued up for stable.
>
> When was the bug added ? This would help a lot stable teams ...
This needs to be backported to 4.16-rc0+, 4.15+, 4.14+, 4.13.14+, and 4.9.63+.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] netlink: put module reference if dump start fails
2018-02-21 14:54 ` Jason A. Donenfeld
@ 2018-02-21 15:11 ` Eric Dumazet
2018-02-21 15:41 ` Jason A. Donenfeld
0 siblings, 1 reply; 8+ messages in thread
From: Eric Dumazet @ 2018-02-21 15:11 UTC (permalink / raw)
To: Jason A. Donenfeld; +Cc: David Miller, Berg, Johannes, Netdev, LKML
On Wed, 2018-02-21 at 15:54 +0100, Jason A. Donenfeld wrote:
> On Wed, Feb 21, 2018 at 6:47 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > > This probably should be queued up for stable.
> >
> > When was the bug added ? This would help a lot stable teams ...
>
> This needs to be backported to 4.16-rc0+, 4.15+, 4.14+, 4.13.14+, and 4.9.63+.
I guess I should have asked you to provide an appropriate Fixes: tag,
clearly identifying the commit that added the bug.
Fixes: 12-digit-sha1 ("patch title")
Thanks.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] netlink: put module reference if dump start fails
2018-02-21 15:11 ` Eric Dumazet
@ 2018-02-21 15:41 ` Jason A. Donenfeld
2018-02-22 6:16 ` Bo YU
0 siblings, 1 reply; 8+ messages in thread
From: Jason A. Donenfeld @ 2018-02-21 15:41 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, Berg, Johannes, Netdev, LKML
Fixes: 41c87425a1ac ("netlink: do not set cb_running if dump's start() errs")
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] netlink: put module reference if dump start fails
2018-02-21 15:41 ` Jason A. Donenfeld
@ 2018-02-22 6:16 ` Bo YU
0 siblings, 0 replies; 8+ messages in thread
From: Bo YU @ 2018-02-22 6:16 UTC (permalink / raw)
To: Jason A. Donenfeld
Cc: Eric Dumazet, David Miller, Berg, Johannes, Netdev, LKML
Hi,
On Wed, Feb 21, 2018 at 04:41:05PM +0100, Jason A. Donenfeld wrote:
>Fixes: 41c87425a1ac ("netlink: do not set cb_running if dump's start() errs")
I think you Would better to resend it.
Bo,
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] netlink: put module reference if dump start fails
2018-02-21 3:41 [PATCH] netlink: put module reference if dump start fails Jason A. Donenfeld
2018-02-21 5:47 ` Eric Dumazet
@ 2018-02-22 19:04 ` David Miller
2018-02-22 22:28 ` Jason A. Donenfeld
1 sibling, 1 reply; 8+ messages in thread
From: David Miller @ 2018-02-22 19:04 UTC (permalink / raw)
To: Jason; +Cc: johannes.berg, netdev, linux-kernel
From: "Jason A. Donenfeld" <Jason@zx2c4.com>
Date: Wed, 21 Feb 2018 04:41:59 +0100
> Before, if cb->start() failed, the module reference would never be put,
> because cb->cb_running is intentionally false at this point. Users are
> generally annoyed by this because they can no longer unload modules that
> leak references. Also, it may be possible to tediously wrap a reference
> counter back to zero, especially since module.c still uses atomic_inc
> instead of refcount_inc.
>
> This patch expands the error path to simply call module_put if
> cb->start() fails.
>
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> ---
> This probably should be queued up for stable.
Applied and queued up for -stable.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-02-22 22:28 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-21 3:41 [PATCH] netlink: put module reference if dump start fails Jason A. Donenfeld
2018-02-21 5:47 ` Eric Dumazet
2018-02-21 14:54 ` Jason A. Donenfeld
2018-02-21 15:11 ` Eric Dumazet
2018-02-21 15:41 ` Jason A. Donenfeld
2018-02-22 6:16 ` Bo YU
2018-02-22 19:04 ` David Miller
2018-02-22 22:28 ` Jason A. Donenfeld
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).