* [PATCH v2] net: rtnetlink: Fix rtnl_dereference may be return NULL
@ 2021-07-08 9:29 Yajun Deng
2021-07-08 9:43 ` Johannes Berg
0 siblings, 1 reply; 4+ messages in thread
From: Yajun Deng @ 2021-07-08 9:29 UTC (permalink / raw)
To: davem, kuba, ryazanov.s.a, johannes.berg, avagin,
vladimir.oltean, cong.wang, roopa, yajun.deng, zhudi21, edumazet
Cc: netdev, linux-kernel
The value 'link' may be NULL in rtnl_unregister(), this leads to
kfree_rcu(NULL, xxx), so add this case handling. And modify the return
value to 'void' in rtnl_unregister(). there is no case using it.
Fixes: addf9b90de22 (net: rtnetlink: use rcu to free rtnl message handlers)
Fixes: 51e13685bd93 (rtnetlink: RCU-annotate both dimensions of rtnl_msg_handlers)
Signed-off-by: Yajun Deng <yajun.deng@linux.dev>
---
include/net/rtnetlink.h | 2 +-
net/core/rtnetlink.c | 18 ++++++++----------
2 files changed, 9 insertions(+), 11 deletions(-)
diff --git a/include/net/rtnetlink.h b/include/net/rtnetlink.h
index 384e800665f2..9d263ad9ea48 100644
--- a/include/net/rtnetlink.h
+++ b/include/net/rtnetlink.h
@@ -17,7 +17,7 @@ void rtnl_register(int protocol, int msgtype,
rtnl_doit_func, rtnl_dumpit_func, unsigned int flags);
int rtnl_register_module(struct module *owner, int protocol, int msgtype,
rtnl_doit_func, rtnl_dumpit_func, unsigned int flags);
-int rtnl_unregister(int protocol, int msgtype);
+void rtnl_unregister(int protocol, int msgtype);
void rtnl_unregister_all(int protocol);
static inline int rtnl_msg_family(const struct nlmsghdr *nlh)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index f6af3e74fc44..e80177c195a5 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -281,10 +281,8 @@ void rtnl_register(int protocol, int msgtype,
* rtnl_unregister - Unregister a rtnetlink message type
* @protocol: Protocol family or PF_UNSPEC
* @msgtype: rtnetlink message type
- *
- * Returns 0 on success or a negative error code.
*/
-int rtnl_unregister(int protocol, int msgtype)
+void rtnl_unregister(int protocol, int msgtype)
{
struct rtnl_link __rcu **tab;
struct rtnl_link *link;
@@ -295,18 +293,18 @@ int rtnl_unregister(int protocol, int msgtype)
rtnl_lock();
tab = rtnl_dereference(rtnl_msg_handlers[protocol]);
- if (!tab) {
- rtnl_unlock();
- return -ENOENT;
- }
+ if (!tab)
+ goto unlock;
link = rtnl_dereference(tab[msgindex]);
- rcu_assign_pointer(tab[msgindex], NULL);
- rtnl_unlock();
+ if (!link)
+ goto unlock;
+ rcu_assign_pointer(tab[msgindex], NULL);
kfree_rcu(link, rcu);
- return 0;
+unlock:
+ rtnl_unlock();
}
EXPORT_SYMBOL_GPL(rtnl_unregister);
--
2.32.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] net: rtnetlink: Fix rtnl_dereference may be return NULL
2021-07-08 9:29 [PATCH v2] net: rtnetlink: Fix rtnl_dereference may be return NULL Yajun Deng
@ 2021-07-08 9:43 ` Johannes Berg
2021-07-08 11:11 ` Vladimir Oltean
0 siblings, 1 reply; 4+ messages in thread
From: Johannes Berg @ 2021-07-08 9:43 UTC (permalink / raw)
To: Yajun Deng, davem, kuba, ryazanov.s.a, avagin, vladimir.oltean,
cong.wang, roopa, zhudi21, edumazet
Cc: netdev, linux-kernel
On Thu, 2021-07-08 at 17:29 +0800, Yajun Deng wrote:
> The value 'link' may be NULL in rtnl_unregister(), this leads to
> kfree_rcu(NULL, xxx), so add this case handling.
>
I don't see how. It would require the caller to unregister something
they never registered. That would be a bug there, but I don't see that
it's very useful to actually be defensive about bugs there.
> And modify the return
> value to 'void' in rtnl_unregister(). there is no case using it.
>
> Fixes: addf9b90de22 (net: rtnetlink: use rcu to free rtnl message handlers)
> Fixes: 51e13685bd93 (rtnetlink: RCU-annotate both dimensions of rtnl_msg_handlers)
It certainly fixes nothing in those patches.
johannes
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] net: rtnetlink: Fix rtnl_dereference may be return NULL
2021-07-08 9:43 ` Johannes Berg
@ 2021-07-08 11:11 ` Vladimir Oltean
2021-07-08 12:26 ` Eric Dumazet
0 siblings, 1 reply; 4+ messages in thread
From: Vladimir Oltean @ 2021-07-08 11:11 UTC (permalink / raw)
To: Johannes Berg
Cc: Yajun Deng, davem, kuba, ryazanov.s.a, avagin, cong.wang, roopa,
zhudi21, edumazet, netdev, linux-kernel
On Thu, Jul 08, 2021 at 11:43:20AM +0200, Johannes Berg wrote:
> On Thu, 2021-07-08 at 17:29 +0800, Yajun Deng wrote:
> > The value 'link' may be NULL in rtnl_unregister(), this leads to
> > kfree_rcu(NULL, xxx), so add this case handling.
> >
>
> I don't see how. It would require the caller to unregister something
> they never registered. That would be a bug there, but I don't see that
> it's very useful to actually be defensive about bugs there.
Besides, isn't kfree_rcu(NULL) safe anyway?
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] net: rtnetlink: Fix rtnl_dereference may be return NULL
2021-07-08 11:11 ` Vladimir Oltean
@ 2021-07-08 12:26 ` Eric Dumazet
0 siblings, 0 replies; 4+ messages in thread
From: Eric Dumazet @ 2021-07-08 12:26 UTC (permalink / raw)
To: Vladimir Oltean, Johannes Berg
Cc: Yajun Deng, davem, kuba, ryazanov.s.a, avagin, cong.wang, roopa,
zhudi21, edumazet, netdev, linux-kernel
On 7/8/21 1:11 PM, Vladimir Oltean wrote:
> On Thu, Jul 08, 2021 at 11:43:20AM +0200, Johannes Berg wrote:
>> On Thu, 2021-07-08 at 17:29 +0800, Yajun Deng wrote:
>>> The value 'link' may be NULL in rtnl_unregister(), this leads to
>>> kfree_rcu(NULL, xxx), so add this case handling.
>>>
>>
>> I don't see how. It would require the caller to unregister something
>> they never registered. That would be a bug there, but I don't see that
>> it's very useful to actually be defensive about bugs there.
>
> Besides, isn't kfree_rcu(NULL) safe anyway?
>
Only from linux-5.3 I think.
(commit 12edff045bc6dd3ab1565cc02fa4841803c2a633 was not backported to old kernels)
But yes, this patch is not solving any bug, as I suspected.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-07-08 12:26 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-08 9:29 [PATCH v2] net: rtnetlink: Fix rtnl_dereference may be return NULL Yajun Deng
2021-07-08 9:43 ` Johannes Berg
2021-07-08 11:11 ` Vladimir Oltean
2021-07-08 12:26 ` Eric Dumazet
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.