mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Stefan Metzmacher <metze@samba.org>
To: Dan Carpenter <dan.carpenter@linaro.org>
Cc: Steve French <sfrench@samba.org>,
	Paulo Alcantara <pc@manguebit.org>,
	Ronnie Sahlberg <ronniesahlberg@gmail.com>,
	Shyam Prasad N <sprasad@microsoft.com>,
	Tom Talpey <tom@talpey.com>, Bharath SM <bharathsm@microsoft.com>,
	linux-cifs@vger.kernel.org, samba-technical@lists.samba.org,
	linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org,
	Namjae Jeon <linkinjeon@kernel.org>
Subject: Using smatch and sparse together (Re: [PATCH next] smb: client: Fix use after free in send_done())
Date: Thu, 7 Aug 2025 08:34:09 +0200	[thread overview]
Message-ID: <df4905fb-933e-4055-8363-d6427515773b@samba.org> (raw)
In-Reply-To: <a1a0046c-f47f-4e8a-ae3c-85db58a6cb2f@suswa.mountain>

Am 06.08.25 um 16:39 schrieb Dan Carpenter:
> On Wed, Aug 06, 2025 at 04:17:41PM +0200, Stefan Metzmacher wrote:
>>>> What was the test that triggered the problem?
>>>> Or did you only noticed it by looking at the code?
>>>
>>> This was a Smatch static checker warning.  You need to have the cross
>>> function DB to detect it.
>>
>> Ok, I'll try to integrate it into my build flow...
>>
>> Does it replace sparse or does it run in addition?
> 
> In addition.  I find the Sparse endianness checks especially useful.
> 
>> If it replaces sparse I guess a small script would
>> run them both?
>>
>> $ cat mychecker.sh:
>> #!/bin/bash
>> set -e
>> sparse $@
>> smatch $@
>>
>> And maybe all others from
>> https://gautammenghani.com/linux,/c/2022/05/19/static-analysis-tools-linux-kernel.html

I'm using this now:

$ cat custom-checker.sh
#!/bin/bash

set -e

which sparse > /dev/null 2>&1 && {
         sparse -Winit-cstring -Wsparse-error $@
}

which smatch > /dev/null 2>&1 && {
         smatch -p=kernel --fatal-checks $@
}

$ cat build-fs-smb.sh
make modules_prepare
make -j16 M=fs/smb CF=-D__CHECK_ENDIAN__ W=1ce C=1 KBUILD_MODPOST_WARN=1 KCFLAGS="-Wfatal-errors" CHECK="$(pwd)/custom-checker.sh" $@


I'm currently getting these warnings:

client/sess.c:436 cifs_chan_update_iface() warn: iterator used outside loop: 'iface'
client/sess.c:444 cifs_chan_update_iface() warn: iterator used outside loop: 'iface'
client/inode.c:1703 cifs_root_iget() warn: passing zero to 'ERR_PTR'
client/inode.c:2295 cifs_mkdir() warn: passing zero to 'ERR_PTR'
server/smb2pdu.c:3754 smb2_open() warn: Function too hairy.  No more merges.
server/smb2pdu.c:3754 smb2_open() parse error: Function too hairy.  Giving up. 18 seconds

Is there a way to use --fatal-checks but turn the 'too hairy' and maybe others into a warning only?
Something like -Wno-error=... in gcc.

Or at least turn this into an error:
client/smbdirect.c:292 send_done() error: dereferencing freed memory 'request' (line 290)
Without --fatal-checks smatch still returns 0.

While this returns an error (without --fatal-checks):
server/smb2pdu.c:3754 smb2_open() warn: Function too hairy.  No more merges.
server/smb2pdu.c:3754 smb2_open() parse error: Function too hairy.  Giving up. 8 seconds

Currently I typically use git rebase -i and then have some like this

exec bash build-fs-smb.sh C=0
pick 123456 my first patch
exec bash build-fs-smb.sh
pick 654321 my 2nd patch
exec bash build-fs-smb.sh

So I force C=0 on the initial run in order to avoid hitting the fatal Function too hairy
and it then works with my default of C=1 if I don't change fs/smb/server/smb2pdu.c
(or with --fatal-checks and other file that has a warning)

I'd actually prefer to use --fatal-checks and C=1 in all cases
in order to notice problems I'm introducing...

>> How often do I need to run smatch_scripts/build_kernel_data.sh on the whole kernel?
> 
> The cross function database is really useful for just information
> purposes and looking at how functions are called.  You probably
> would need to rebuild it four or five times to get useful
> information, unfortunately.  I rebuild my every night on the latest
> linux-next.

I have the following files generated on a fast machine:

$ ls -alrt smatch_*
-rw-r----- 1 metze metze     303104 Aug  6 15:42 smatch_db.sqlite.new
-rw-rw-r-- 1 metze metze    3107065 Aug  6 16:37 smatch_compile.warns
-rw-rw-r-- 1 metze metze 2848012813 Aug  6 16:37 smatch_warns.txt
-rw-rw-r-- 1 metze metze 6016192672 Aug  6 16:38 smatch_warns.txt.sql
-rw-rw-r-- 1 metze metze 4202917492 Aug  6 16:39 smatch_warns.txt.caller_info
-rw-r--r-- 1 metze metze 8757637120 Aug  6 16:57 smatch_db.sqlite

I copied them all to my laptop where I develop my patches
and was able to reproduce the error :-)

Do I need copy all of these or is smatch_db.sqlite enough?

Would it be possible that you share your generated file(s)
via a download, that might be useful for a lot of people.

Anyway thanks for all the hints so far:-)
metze


  reply	other threads:[~2025-08-07  6:34 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-06 11:45 [PATCH next] smb: client: Fix use after free in send_done() Dan Carpenter
2025-08-06 12:20 ` Stefan Metzmacher
2025-08-06 12:48   ` Dan Carpenter
2025-08-06 14:17     ` Stefan Metzmacher
2025-08-06 14:39       ` Dan Carpenter
2025-08-07  6:34         ` Stefan Metzmacher [this message]
2025-08-07  7:22           ` Using smatch and sparse together (Re: [PATCH next] smb: client: Fix use after free in send_done()) Dan Carpenter
2025-08-07 14:27             ` Stefan Metzmacher
2025-08-07 15:06               ` Dan Carpenter
2025-08-07 15:17                 ` Stefan Metzmacher
2025-08-07 15:36                   ` Dan Carpenter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=df4905fb-933e-4055-8363-d6427515773b@samba.org \
    --to=metze@samba.org \
    --cc=bharathsm@microsoft.com \
    --cc=dan.carpenter@linaro.org \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linkinjeon@kernel.org \
    --cc=linux-cifs@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pc@manguebit.org \
    --cc=ronniesahlberg@gmail.com \
    --cc=samba-technical@lists.samba.org \
    --cc=sfrench@samba.org \
    --cc=sprasad@microsoft.com \
    --cc=tom@talpey.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®