* [PATCH next] smb: client: Fix use after free in send_done()
@ 2025-08-06 11:45 Dan Carpenter
2025-08-06 12:20 ` Stefan Metzmacher
0 siblings, 1 reply; 11+ messages in thread
From: Dan Carpenter @ 2025-08-06 11:45 UTC (permalink / raw)
To: Stefan Metzmacher
Cc: Steve French, Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N,
Tom Talpey, Bharath SM, linux-cifs, samba-technical, linux-kernel,
kernel-janitors
The mempool_free() function frees "request". Don't free the request
until after smbd_disconnect_rdma_connection() to avoid a use after free
bug.
Fixes: 5e65668c75c0 ("smb: client: let send_done() cleanup before calling smbd_disconnect_rdma_connection()")
Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
---
fs/smb/client/smbdirect.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/smb/client/smbdirect.c b/fs/smb/client/smbdirect.c
index 58321e483a1a..162f8d1c548a 100644
--- a/fs/smb/client/smbdirect.c
+++ b/fs/smb/client/smbdirect.c
@@ -286,8 +286,8 @@ static void send_done(struct ib_cq *cq, struct ib_wc *wc)
if (wc->status != IB_WC_SUCCESS || wc->opcode != IB_WC_SEND) {
log_rdma_send(ERR, "wc->status=%d wc->opcode=%d\n",
wc->status, wc->opcode);
- mempool_free(request, request->info->request_mempool);
smbd_disconnect_rdma_connection(request->info);
+ mempool_free(request, request->info->request_mempool);
return;
}
--
2.47.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH next] smb: client: Fix use after free in send_done()
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
0 siblings, 1 reply; 11+ messages in thread
From: Stefan Metzmacher @ 2025-08-06 12:20 UTC (permalink / raw)
To: Dan Carpenter
Cc: Steve French, Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N,
Tom Talpey, Bharath SM, linux-cifs, samba-technical, linux-kernel,
kernel-janitors
Hi Dan,
> The mempool_free() function frees "request". Don't free the request
> until after smbd_disconnect_rdma_connection() to avoid a use after free
> bug.
>
> Fixes: 5e65668c75c0 ("smb: client: let send_done() cleanup before calling smbd_disconnect_rdma_connection()")
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> ---
> fs/smb/client/smbdirect.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/smb/client/smbdirect.c b/fs/smb/client/smbdirect.c
> index 58321e483a1a..162f8d1c548a 100644
> --- a/fs/smb/client/smbdirect.c
> +++ b/fs/smb/client/smbdirect.c
> @@ -286,8 +286,8 @@ static void send_done(struct ib_cq *cq, struct ib_wc *wc)
> if (wc->status != IB_WC_SUCCESS || wc->opcode != IB_WC_SEND) {
> log_rdma_send(ERR, "wc->status=%d wc->opcode=%d\n",
> wc->status, wc->opcode);
> - mempool_free(request, request->info->request_mempool);
> smbd_disconnect_rdma_connection(request->info);
> + mempool_free(request, request->info->request_mempool);
The correct fix is to use 'info' instead of 'request->info'
other than that the order needs to stay that way.
I already asked Steve to squash such a change into the
original commit (which is not yet upstream).
See:
https://lore.kernel.org/linux-cifs/cover.1754308712.git.metze@samba.org/T/#m98a8607d7b83a11fd78547306836a872a2a27192
What was the test that triggered the problem?
Or did you only noticed it by looking at the code?
Thanks!
metze
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH next] smb: client: Fix use after free in send_done()
2025-08-06 12:20 ` Stefan Metzmacher
@ 2025-08-06 12:48 ` Dan Carpenter
2025-08-06 14:17 ` Stefan Metzmacher
0 siblings, 1 reply; 11+ messages in thread
From: Dan Carpenter @ 2025-08-06 12:48 UTC (permalink / raw)
To: Stefan Metzmacher
Cc: Steve French, Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N,
Tom Talpey, Bharath SM, linux-cifs, samba-technical, linux-kernel,
kernel-janitors
On Wed, Aug 06, 2025 at 02:20:56PM +0200, Stefan Metzmacher wrote:
> Hi Dan,
>
> > The mempool_free() function frees "request". Don't free the request
> > until after smbd_disconnect_rdma_connection() to avoid a use after free
> > bug.
> >
> > Fixes: 5e65668c75c0 ("smb: client: let send_done() cleanup before calling smbd_disconnect_rdma_connection()")
> > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> > ---
> > fs/smb/client/smbdirect.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/fs/smb/client/smbdirect.c b/fs/smb/client/smbdirect.c
> > index 58321e483a1a..162f8d1c548a 100644
> > --- a/fs/smb/client/smbdirect.c
> > +++ b/fs/smb/client/smbdirect.c
> > @@ -286,8 +286,8 @@ static void send_done(struct ib_cq *cq, struct ib_wc *wc)
> > if (wc->status != IB_WC_SUCCESS || wc->opcode != IB_WC_SEND) {
> > log_rdma_send(ERR, "wc->status=%d wc->opcode=%d\n",
> > wc->status, wc->opcode);
> > - mempool_free(request, request->info->request_mempool);
> > smbd_disconnect_rdma_connection(request->info);
> > + mempool_free(request, request->info->request_mempool);
>
> The correct fix is to use 'info' instead of 'request->info'
> other than that the order needs to stay that way.
>
> I already asked Steve to squash such a change into the
> original commit (which is not yet upstream).
>
> See:
> https://lore.kernel.org/linux-cifs/cover.1754308712.git.metze@samba.org/T/#m98a8607d7b83a11fd78547306836a872a2a27192
>
> 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.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH next] smb: client: Fix use after free in send_done()
2025-08-06 12:48 ` Dan Carpenter
@ 2025-08-06 14:17 ` Stefan Metzmacher
2025-08-06 14:39 ` Dan Carpenter
0 siblings, 1 reply; 11+ messages in thread
From: Stefan Metzmacher @ 2025-08-06 14:17 UTC (permalink / raw)
To: Dan Carpenter
Cc: Steve French, Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N,
Tom Talpey, Bharath SM, linux-cifs, samba-technical, linux-kernel,
kernel-janitors
Am 06.08.25 um 14:48 schrieb Dan Carpenter:
> On Wed, Aug 06, 2025 at 02:20:56PM +0200, Stefan Metzmacher wrote:
>> Hi Dan,
>>
>>> The mempool_free() function frees "request". Don't free the request
>>> until after smbd_disconnect_rdma_connection() to avoid a use after free
>>> bug.
>>>
>>> Fixes: 5e65668c75c0 ("smb: client: let send_done() cleanup before calling smbd_disconnect_rdma_connection()")
>>> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
>>> ---
>>> fs/smb/client/smbdirect.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/fs/smb/client/smbdirect.c b/fs/smb/client/smbdirect.c
>>> index 58321e483a1a..162f8d1c548a 100644
>>> --- a/fs/smb/client/smbdirect.c
>>> +++ b/fs/smb/client/smbdirect.c
>>> @@ -286,8 +286,8 @@ static void send_done(struct ib_cq *cq, struct ib_wc *wc)
>>> if (wc->status != IB_WC_SUCCESS || wc->opcode != IB_WC_SEND) {
>>> log_rdma_send(ERR, "wc->status=%d wc->opcode=%d\n",
>>> wc->status, wc->opcode);
>>> - mempool_free(request, request->info->request_mempool);
>>> smbd_disconnect_rdma_connection(request->info);
>>> + mempool_free(request, request->info->request_mempool);
>>
>> The correct fix is to use 'info' instead of 'request->info'
>> other than that the order needs to stay that way.
>>
>> I already asked Steve to squash such a change into the
>> original commit (which is not yet upstream).
>>
>> See:
>> https://lore.kernel.org/linux-cifs/cover.1754308712.git.metze@samba.org/T/#m98a8607d7b83a11fd78547306836a872a2a27192
>>
>> 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?
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
How often do I need to run smatch_scripts/build_kernel_data.sh on the whole kernel?
Thanks!
metze
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH next] smb: client: Fix use after free in send_done()
2025-08-06 14:17 ` Stefan Metzmacher
@ 2025-08-06 14:39 ` Dan Carpenter
2025-08-07 6:34 ` Using smatch and sparse together (Re: [PATCH next] smb: client: Fix use after free in send_done()) Stefan Metzmacher
0 siblings, 1 reply; 11+ messages in thread
From: Dan Carpenter @ 2025-08-06 14:39 UTC (permalink / raw)
To: Stefan Metzmacher
Cc: Steve French, Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N,
Tom Talpey, Bharath SM, linux-cifs, samba-technical, linux-kernel,
kernel-janitors
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
>
> 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.
But for other people, I normally say don't bother with the cross
function DB. It takes a long time to build and it only slightly
improves the output.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 11+ messages in thread
* Using smatch and sparse together (Re: [PATCH next] smb: client: Fix use after free in send_done())
2025-08-06 14:39 ` Dan Carpenter
@ 2025-08-07 6:34 ` Stefan Metzmacher
2025-08-07 7:22 ` Dan Carpenter
0 siblings, 1 reply; 11+ messages in thread
From: Stefan Metzmacher @ 2025-08-07 6:34 UTC (permalink / raw)
To: Dan Carpenter
Cc: Steve French, Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N,
Tom Talpey, Bharath SM, linux-cifs, samba-technical, linux-kernel,
kernel-janitors, Namjae Jeon
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Using smatch and sparse together (Re: [PATCH next] smb: client: Fix use after free in send_done())
2025-08-07 6:34 ` Using smatch and sparse together (Re: [PATCH next] smb: client: Fix use after free in send_done()) Stefan Metzmacher
@ 2025-08-07 7:22 ` Dan Carpenter
2025-08-07 14:27 ` Stefan Metzmacher
0 siblings, 1 reply; 11+ messages in thread
From: Dan Carpenter @ 2025-08-07 7:22 UTC (permalink / raw)
To: Stefan Metzmacher
Cc: Steve French, Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N,
Tom Talpey, Bharath SM, linux-cifs, samba-technical, linux-kernel,
kernel-janitors, Namjae Jeon
On Thu, Aug 07, 2025 at 08:34:09AM +0200, Stefan Metzmacher wrote:
> 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 $@
I would say don't do fatal checks... I don't love that option at all.
It was for another project which limits which checks it enables.
> }
>
> $ 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'
This code is fine. It's quite hard for Smatch to parse
if (list_entry_is_head(iface, &ses->iface_list, iface_head)) {
correctly.
> client/inode.c:1703 cifs_root_iget() warn: passing zero to 'ERR_PTR'
Huh... This warning showed up in 2023 and I didn't report it. I
probably should have.
fs/smb/client/inode.c
1693 iget_root:
1694 if (!rc) {
1695 if (fattr.cf_flags & CIFS_FATTR_JUNCTION) {
1696 fattr.cf_flags &= ~CIFS_FATTR_JUNCTION;
1697 cifs_autodisable_serverino(cifs_sb);
1698 }
1699 inode = cifs_iget(sb, &fattr);
Should this have been:
inode = cifs_iget(sb, &fattr);
if (!inode)
rc = -EINVAL;
1700 }
1701
1702 if (!inode) {
1703 inode = ERR_PTR(rc);
1704 goto out;
1705 }
> client/inode.c:2295 cifs_mkdir() warn: passing zero to 'ERR_PTR'
Returning ERR_PTR(0) means reporting NULL and it's an idiom in fs/.
But outside of fs/ then most times it is a bug. So the warning is
useful, but in fs/ it's often deliberate like it is here.
> 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
>
Yeah. Ignore these.
> 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.
Yeah. Let me disable those unless --spammy is enabled. They're for
debugging only and I'm probably the only person who is interested in
them.
>
> 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.
>
Sure. To be honest, I normally build with the --succeed option which
is the opposite of --fatal-checks.
> 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...
I use the scripts/new_bugs.pl script. After I've looked at the day's
warnings then I run `scripts/new_bugs.pl --store err-list` and only
review them again when I modify a file.
>
> > > 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
>
Your DB is 8GB. If you rebuild it enough times, then eventually the
DB will max out at 32GB. If it gets to be over 40GB then my builds
stop finishing in one night so I investigate and shrink it again...
It's a cycle of adding code until things slow down too much and then
optimizing it to make it bearable again.
> 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?
Yep. Only smatch_db.sqlite. I should probably delete the other
files after the DB has been built.
>
> Would it be possible that you share your generated file(s)
> via a download, that might be useful for a lot of people.
>
The DB is too big and too dependent on your .config but I should
share the smatch_data/ more regularly. I started to push that into
a separate git repo but I didn't finish that work. I should do
that.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Using smatch and sparse together (Re: [PATCH next] smb: client: Fix use after free in send_done())
2025-08-07 7:22 ` Dan Carpenter
@ 2025-08-07 14:27 ` Stefan Metzmacher
2025-08-07 15:06 ` Dan Carpenter
0 siblings, 1 reply; 11+ messages in thread
From: Stefan Metzmacher @ 2025-08-07 14:27 UTC (permalink / raw)
To: Dan Carpenter
Cc: Steve French, Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N,
Tom Talpey, Bharath SM, linux-cifs, samba-technical, linux-kernel,
kernel-janitors, Namjae Jeon
Am 07.08.25 um 09:22 schrieb Dan Carpenter:
> On Thu, Aug 07, 2025 at 08:34:09AM +0200, Stefan Metzmacher wrote:
>> 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:
This seems to work for me now:
$ cat custom-checker.sh
#!/bin/bash
set -e
which sparse > /dev/null 2>&1 && {
sparse -Winit-cstring -Wsparse-error -fdiagnostic-prefix=SPARSE $@
}
which smatch > /dev/null 2>&1 && {
smatch -p=kernel --pedantic --succeed $@
}
$ cat build-fs-smb.sh
#!/bin/bash
#
set -ueo pipefail
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" $@ 2>&1 | tee build-fs-smb.out
cat build-fs-smb.out | grep -v 'parse error: Function too hairy' | grep -q 'error:' || {
rm build-fs-smb.out
exit 0
}
echo ""
echo "BUILD-ERRORS:"
cat build-fs-smb.out | grep -v 'parse error: Function too hairy' | grep 'error:'
find fs/smb -name '*.o' | xargs rm
find fs/smb -name '*.ko' | xargs rm
rm build-fs-smb.out
exit 1
> The DB is too big and too dependent on your .config but I should
> share the smatch_data/ more regularly. I started to push that into
> a separate git repo but I didn't finish that work. I should do
> that.
Ok, what's the gain of updating it?
Does it help when doing fixes on old kernels?
I'm typically doing a full kernel build a week after each rc.
My idea was to rebuild the whole db after doing that.
Thanks!
metze
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Using smatch and sparse together (Re: [PATCH next] smb: client: Fix use after free in send_done())
2025-08-07 14:27 ` Stefan Metzmacher
@ 2025-08-07 15:06 ` Dan Carpenter
2025-08-07 15:17 ` Stefan Metzmacher
0 siblings, 1 reply; 11+ messages in thread
From: Dan Carpenter @ 2025-08-07 15:06 UTC (permalink / raw)
To: Stefan Metzmacher
Cc: Steve French, Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N,
Tom Talpey, Bharath SM, linux-cifs, samba-technical, linux-kernel,
kernel-janitors, Namjae Jeon
On Thu, Aug 07, 2025 at 04:27:41PM +0200, Stefan Metzmacher wrote:
> > The DB is too big and too dependent on your .config but I should
> > share the smatch_data/ more regularly. I started to push that into
> > a separate git repo but I didn't finish that work. I should do
> > that.
>
> Ok, what's the gain of updating it?
> Does it help when doing fixes on old kernels?
If you run smatch_scripts/build_kernel_data.sh then it runs
smatch_scripts/gen_* which generates a bunch of the files in
smatch_data/. Which in theory should enable more warnings for new
code.
I've been moving away from generating files and more towards
putting everything in the DB. I just took a look at the files now
to respond to your email and what I saw wasn't good... I need to
look at this some more.
I don't know how often the zero day bot rebuilds the smatch_data.
I bet they never do and so I think it doesn't really matter.
>
> I'm typically doing a full kernel build a week after each rc.
> My idea was to rebuild the whole db after doing that.
Yeah. That's a good strategy. The data from the existing DB feeds
into the new one when you rebuild the DB so don't delete the old
DB at the start or anything.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Using smatch and sparse together (Re: [PATCH next] smb: client: Fix use after free in send_done())
2025-08-07 15:06 ` Dan Carpenter
@ 2025-08-07 15:17 ` Stefan Metzmacher
2025-08-07 15:36 ` Dan Carpenter
0 siblings, 1 reply; 11+ messages in thread
From: Stefan Metzmacher @ 2025-08-07 15:17 UTC (permalink / raw)
To: Dan Carpenter
Cc: Steve French, Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N,
Tom Talpey, Bharath SM, linux-cifs, samba-technical, linux-kernel,
kernel-janitors, Namjae Jeon
Am 07.08.25 um 17:06 schrieb Dan Carpenter:
> On Thu, Aug 07, 2025 at 04:27:41PM +0200, Stefan Metzmacher wrote:
>>> The DB is too big and too dependent on your .config but I should
>>> share the smatch_data/ more regularly. I started to push that into
>>> a separate git repo but I didn't finish that work. I should do
>>> that.
>>
>> Ok, what's the gain of updating it?
>> Does it help when doing fixes on old kernels?
>
> If you run smatch_scripts/build_kernel_data.sh then it runs
> smatch_scripts/gen_* which generates a bunch of the files in
> smatch_data/. Which in theory should enable more warnings for new
> code.
>
> I've been moving away from generating files and more towards
> putting everything in the DB. I just took a look at the files now
> to respond to your email and what I saw wasn't good... I need to
> look at this some more.
>
> I don't know how often the zero day bot rebuilds the smatch_data.
> I bet they never do and so I think it doesn't really matter.
>
>>
>> I'm typically doing a full kernel build a week after each rc.
>> My idea was to rebuild the whole db after doing that.
>
> Yeah. That's a good strategy. The data from the existing DB feeds
> into the new one when you rebuild the DB so don't delete the old
> DB at the start or anything.
I mean I'm typically do a git clean -xdf . in order
to wipe everything in order to do a clean:
make -j33 bindeb-pkg
So it would build a new DB, as I'm working based on
the new kernel I guess that's all I need or
are there other reasons to update the existing DB?
Thanks!
metze
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Using smatch and sparse together (Re: [PATCH next] smb: client: Fix use after free in send_done())
2025-08-07 15:17 ` Stefan Metzmacher
@ 2025-08-07 15:36 ` Dan Carpenter
0 siblings, 0 replies; 11+ messages in thread
From: Dan Carpenter @ 2025-08-07 15:36 UTC (permalink / raw)
To: Stefan Metzmacher
Cc: Steve French, Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N,
Tom Talpey, Bharath SM, linux-cifs, samba-technical, linux-kernel,
kernel-janitors, Namjae Jeon
On Thu, Aug 07, 2025 at 05:17:05PM +0200, Stefan Metzmacher wrote:
> > > I'm typically doing a full kernel build a week after each rc.
> > > My idea was to rebuild the whole db after doing that.
> >
> > Yeah. That's a good strategy. The data from the existing DB feeds
> > into the new one when you rebuild the DB so don't delete the old
> > DB at the start or anything.
>
> I mean I'm typically do a git clean -xdf . in order
> to wipe everything in order to do a clean:
> make -j33 bindeb-pkg
>
> So it would build a new DB, as I'm working based on
> the new kernel I guess that's all I need or
> are there other reasons to update the existing DB?
You really want to keep the old smatch_db.sqlite DB file between
rebuilds. Every time you rebuild the DB it adds more information to
the call tree.
Imagine a() passes a number to b() which passes it to c(). It takes
two rebuilds for that information to be built out.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2025-08-07 15:36 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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 ` Using smatch and sparse together (Re: [PATCH next] smb: client: Fix use after free in send_done()) Stefan Metzmacher
2025-08-07 7:22 ` 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
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).