netdev - Re: [syzbot] [can?] WARNING: refcount bug in j1939_session

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZrMqFN4vE7WHRBjE@gmail.com>
Date: Wed, 7 Aug 2024 01:02:28 -0700
From: Breno Leitao <leitao@...ian.org>
To: Edward Adam Davis <eadavis@...com>
Cc: syzbot+ad601904231505ad6617@...kaller.appspotmail.com,
	davem@...emloft.net, edumazet@...gle.com, kernel@...gutronix.de,
	kuba@...nel.org, linux-can@...r.kernel.org,
	linux-kernel@...r.kernel.org, mkl@...gutronix.de,
	netdev@...r.kernel.org, o.rempel@...gutronix.de, pabeni@...hat.com,
	robin@...tonic.nl, socketcan@...tkopp.net,
	syzkaller-bugs@...glegroups.com
Subject: Re: [syzbot] [can?] WARNING: refcount bug in j1939_session_put

Hello Edward,

On Wed, Aug 07, 2024 at 09:42:40AM +0800, Edward Adam Davis wrote:
> Fixes: c9c0ee5f20c5 ("net: skbuff: Skip early return in skb_unref when debugging")
> 
> Root cause: In commit c9c0ee5f20c5, There are following rules:
> In debug builds (CONFIG_DEBUG_NET set), the reference count is always  decremented, even when it's 1

That is the goal, to pick problems like the one reported here. I.e, the
reference shouldn't be negative. If that is the case, it means that
there is a bug, and the skb is being unreferenced more than what it
needs to.

> This rule will cause the reference count to be 0 after calling skc_unref,
> which will affect the release of skb.
> 
> The solution I have proposed is:
> Before releasing the SKB during session destroy, check the CONFIG_DEBUG_NET
> and skb_unref return values to avoid reference count errors caused by a 
> reference count of 0 when releasing the SKB.

I am not sure this is the best approach. I would sugest finding where
the skb is being unreferenced first, so, it doesn't need to be
unreferenced again.

This suggestion is basically working around the findings.

Thanks for looking at this problem.
--breno