[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <87bjreuhw8.fsf@posteo.net>
Date: Tue, 27 May 2025 16:39:22 +0000
From: Charalampos Mitrodimas <charmitro@...teo.net>
To: Tung Quang Nguyen <tung.quang.nguyen@....tech>
Cc: Jon Maloy <jmaloy@...hat.com>, "David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>, Simon Horman <horms@...nel.org>, Wang
Liang <wangliang74@...wei.com>, "netdev@...r.kernel.org"
<netdev@...r.kernel.org>, "tipc-discussion@...ts.sourceforge.net"
<tipc-discussion@...ts.sourceforge.net>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>,
"syzbot+f0c4a4aba757549ae26c@...kaller.appspotmail.com"
<syzbot+f0c4a4aba757549ae26c@...kaller.appspotmail.com>
Subject: Re: [PATCH net] net: tipc: fix refcount warning in tipc_aead_encrypt
Tung Quang Nguyen <tung.quang.nguyen@....tech> writes:
>>Subject: [PATCH net] net: tipc: fix refcount warning in tipc_aead_encrypt
>>
>>syzbot reported a refcount warning [1] caused by calling get_net() on a
>>network namespace that is being destroyed (refcount=0). This happens when a
>>TIPC discovery timer fires during network namespace cleanup.
>>
>>The recently added get_net() call in commit e279024617134 ("net/tipc:
>>fix slab-use-after-free Read in tipc_aead_encrypt_done") attempts to hold a
>>reference to the network namespace. However, if the namespace is already
>>being destroyed, its refcount might be zero, leading to the use-after-free
>>warning.
>>
>>Replace get_net() with maybe_get_net(), which safely checks if the refcount is
>>non-zero before incrementing it. If the namespace is being destroyed, return -
>>ENXIO early, after releasing the bearer reference.
>>
>>[1]:
>>https://lore.kernel.org/all/68342b55.a70a0220.253bc2.0091.GAE@google.com
>>/T/#m12019cf9ae77e1954f666914640efa36d52704a2
>>
>>Reported-by: syzbot+f0c4a4aba757549ae26c@...kaller.appspotmail.com
>>Closes:
>>https://lore.kernel.org/all/68342b55.a70a0220.253bc2.0091.GAE@google.com
>>/T/#m12019cf9ae77e1954f666914640efa36d52704a2
>>Fixes: e27902461713 ("net/tipc: fix slab-use-after-free Read in
>>tipc_aead_encrypt_done")
>>Signed-off-by: Charalampos Mitrodimas <charmitro@...teo.net>
>>---
>> net/tipc/crypto.c | 6 +++++-
>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>
>>diff --git a/net/tipc/crypto.c b/net/tipc/crypto.c index
>>8584893b478510dc1ddda321ed06054de327609b..49916f983fe5e1d484779451
>>04fe5fc589257533 100644
>>--- a/net/tipc/crypto.c
>>+++ b/net/tipc/crypto.c
>>@@ -818,7 +818,11 @@ static int tipc_aead_encrypt(struct tipc_aead *aead,
>>struct sk_buff *skb,
>> }
>>
>> /* Get net to avoid freed tipc_crypto when delete namespace */
>>- get_net(aead->crypto->net);
>>+ if (!maybe_get_net(aead->crypto->net)) {
>>+ tipc_bearer_put(b);
>>+ rc = -ENXIO;
> -ENODEV should be used instead as we also use it for bearer ref count. Thus, caller of tipc_aead_encrypt() does not need to care about handling new error code.
Hi,
Thanks for the review!
Addressed in v2.
C. Mitrodimas
>>+ goto exit;
>>+ }
>>
>> /* Now, do encrypt */
>> rc = crypto_aead_encrypt(req);
Powered by blists - more mailing lists