Smatch mailing list archives

Smatch mailing list archives

[Reshetova, Elena]

Hi, 

I have been working for a while now on a new smatch pattern, but
would really appreciate additional information points such as past
email discussions, etc. 

So I am wondering if there is a way to browse through
the archives of this mailing list in order to try to find the
information I need? 

Best Regards,
Elena.

Re: Smatch mailing list archives

[Dan Carpenter]

On Fri, May 07, 2021 at 02:22:37PM +0000, Reshetova, Elena wrote:
> Hi, 
> 
> I have been working for a while now on a new smatch pattern, but
> would really appreciate additional information points such as past
> email discussions, etc. 
> 
> So I am wondering if there is a way to browse through
> the archives of this mailing list in order to try to find the
> information I need? 

Sorry, I don't think it's archived anywhere.  There isn't a lot of
traffic on the list.  About three times a year someone reports that
Smatch is crashing for them.

I'm always happy to answer questions if there is any way I can help?

regards,
dan carpenter

RE: Smatch mailing list archives

[Reshetova, Elena]

> On Fri, May 07, 2021 at 02:22:37PM +0000, Reshetova, Elena wrote:
> > Hi,
> >
> > I have been working for a while now on a new smatch pattern, but
> > would really appreciate additional information points such as past
> > email discussions, etc.
> >
> > So I am wondering if there is a way to browse through
> > the archives of this mailing list in order to try to find the
> > information I need?
> 
> Sorry, I don't think it's archived anywhere.  There isn't a lot of
> traffic on the list.  About three times a year someone reports that
> Smatch is crashing for them.
> 
> I'm always happy to answer questions if there is any way I can help?

Thank you Dan! I am pretty new with smatch so that's why I was
hoping to browse through the existing mails to see if my simple questions
are already answered, but here is my current issue. 

What is the best way to create identifiers for the findings that certain smatch
pattern finds in the kernel? Let's say I have a new pattern that is able to find
different problematic places and report them in usual smatch way: errors and
warnings with file name, line number, function name, etc. 
Now for our pattern in order to be sure that the reported issue exists/does not
exists, somebody needs to go and look at the code manually and make a call. 
After this, it would be nice to mark this place as safe/concern in the report and be
able to transfer these results for kernel versions bumps (5.11->5.12, etc.) as soon as
the code in this function where finding was reported has not changed (and there
might be multiple findings per function).

What is the best way of doing it? 
I was first thinking of using some simple hash for the reported line (lines around, relative
position within the reported function),
but now I think I need also to hash the whole function in addition to the finding itself. 

Then the logic of transferring the result would be:

For each finding calculate: 
 1. finding_line_hash: the hash of the line that resulted in finding (becomes a unique id
     within the function).
2. finding_function_hash: the hash of the function that produced the finding (becomes a
   unique global id within the kernel) and helps to determine if the function has not been
   changed between the kernel versions. 

Logic for the result transfer: 

If both finding_line_hash and finding_function_hash match between the two smatch reports
for two different versions, then it is relatively safe to transfer this concrete smatch finding
and its manual audit result automatically.

Does it make sense overall? If yes, what is the easiest way in smatch to get hash data for 
1 and 2? I.e. get full reported line as a string and full function content as a string? 

Best Regards,
Elena.


  

Re: Smatch mailing list archives

[Oleg Drokin]

> On May 10, 2021, at 2:17 AM, Reshetova, Elena <elena.reshetova@intel.com> wrote:
> 
> What is the best way to create identifiers for the findings that certain smatch
> pattern finds in the kernel? Let's say I have a new pattern that is able to find
> different problematic places and report them in usual smatch way: errors and
> warnings with file name, line number, function name, etc. 
> Now for our pattern in order to be sure that the reported issue exists/does not
> exists, somebody needs to go and look at the code manually and make a call. 
> After this, it would be nice to mark this place as safe/concern in the report and be
> able to transfer these results for kernel versions bumps (5.11->5.12, etc.) as soon as
> the code in this function where finding was reported has not changed (and there
> might be multiple findings per function).

When I was doing a similar thing to integrate smatch into gerrit, I decided that
line numbers are too unreliable a metric to really depend on.
Instead for most cases just using the rest of the message seems to be good enough?
Function name, whatever variable or the message there is and such is used to filter out
known failures (two kinds: false positives and actual bugs that need to be fixed, but
are yet unfixed so no point in alerting people to them again and again on unrelated
review requests)

Seems to be working well though I am sure might not be super perfect.

In the end I guess your end goal is unclear? Once you have those results from two
different kernel versions and there are some minute changes, are you just trying to
avoid retriaging things in unchanged code only? That might be quite a burden
if you have lots of false positives, for example.

RE: Smatch mailing list archives

[Reshetova, Elena]

> > On May 10, 2021, at 2:17 AM, Reshetova, Elena <elena.reshetova@intel.com>
> wrote:
> >
> > What is the best way to create identifiers for the findings that certain smatch
> > pattern finds in the kernel? Let's say I have a new pattern that is able to find
> > different problematic places and report them in usual smatch way: errors and
> > warnings with file name, line number, function name, etc.
> > Now for our pattern in order to be sure that the reported issue exists/does not
> > exists, somebody needs to go and look at the code manually and make a call.
> > After this, it would be nice to mark this place as safe/concern in the report and be
> > able to transfer these results for kernel versions bumps (5.11->5.12, etc.) as soon
> as
> > the code in this function where finding was reported has not changed (and there
> > might be multiple findings per function).
> 
> When I was doing a similar thing to integrate smatch into gerrit, I decided that
> line numbers are too unreliable a metric to really depend on.
> Instead for most cases just using the rest of the message seems to be good enough?
> Function name, whatever variable or the message there is and such is used to filter
> out
> known failures (two kinds: false positives and actual bugs that need to be fixed, but
> are yet unfixed so no point in alerting people to them again and again on unrelated
> review requests)
> 
> Seems to be working well though I am sure might not be super perfect.

I don't think this would be good enough for our case, since what we essentially looking 
for with smatch is a set of function calls to a specific functions. So, I end up with 
smth like:

drivers/vme/boards/vme_vmivme7805.c:69 vmic_probe() warn: read using function
 'ioread32' to an int type local variable 'data', type is uint
drivers/vme/boards/vme_vmivme7805.c:74 vmic_probe() warn: read using function
 'ioread32' to an int type local variable 'data', type is uint

The two above items are in the same function vmic_probe(), making the same function
call " ioread32" and writing down the result of this function call into the same var 'data'.
They just happen to do this in two different places in vmic_probe function. 
So, essentially they are even exactly the same lines in this case and the only difference is
relative line position within the analyzed vmic_probe() function. 

> 
> In the end I guess your end goal is unclear? Once you have those results from two
> different kernel versions and there are some minute changes, are you just trying to
> avoid retriaging things in unchanged code only? That might be quite a burden
> if you have lots of false positives, for example.

Yes, I want to avoid retriaging the things that have been triaged before and if the code
has not been changed within the single function. 

Best Regards,
Elena.

Re: Smatch mailing list archives

[Oleg Drokin]

> On May 10, 2021, at 5:31 AM, Reshetova, Elena <elena.reshetova@intel.com> wrote:
> 
>>> On May 10, 2021, at 2:17 AM, Reshetova, Elena <elena.reshetova@intel.com>
>> wrote:
>>> 
>>> What is the best way to create identifiers for the findings that certain smatch
>>> pattern finds in the kernel? Let's say I have a new pattern that is able to find
>>> different problematic places and report them in usual smatch way: errors and
>>> warnings with file name, line number, function name, etc.
>>> Now for our pattern in order to be sure that the reported issue exists/does not
>>> exists, somebody needs to go and look at the code manually and make a call.
>>> After this, it would be nice to mark this place as safe/concern in the report and be
>>> able to transfer these results for kernel versions bumps (5.11->5.12, etc.) as soon
>> as
>>> the code in this function where finding was reported has not changed (and there
>>> might be multiple findings per function).
>> 
>> When I was doing a similar thing to integrate smatch into gerrit, I decided that
>> line numbers are too unreliable a metric to really depend on.
>> Instead for most cases just using the rest of the message seems to be good enough?
>> Function name, whatever variable or the message there is and such is used to filter
>> out
>> known failures (two kinds: false positives and actual bugs that need to be fixed, but
>> are yet unfixed so no point in alerting people to them again and again on unrelated
>> review requests)
>> 
>> Seems to be working well though I am sure might not be super perfect.
> 
> I don't think this would be good enough for our case, since what we essentially looking 
> for with smatch is a set of function calls to a specific functions. So, I end up with 
> smth like:
> 
> drivers/vme/boards/vme_vmivme7805.c:69 vmic_probe() warn: read using function
> 'ioread32' to an int type local variable 'data', type is uint
> drivers/vme/boards/vme_vmivme7805.c:74 vmic_probe() warn: read using function
> 'ioread32' to an int type local variable 'data', type is uint
> 
> The two above items are in the same function vmic_probe(), making the same function
> call " ioread32" and writing down the result of this function call into the same var 'data'.
> They just happen to do this in two different places in vmic_probe function. 
> So, essentially they are even exactly the same lines in this case and the only difference is
> relative line position within the analyzed vmic_probe() function. 

So is your concern that one of those warnings might be a real bug while another one
is not? Because if they both are correct or both are incorrect seemingly
it does not matter (for filtering purposes) that you have two instead of one?

Re: Smatch mailing list archives

[Dan Carpenter]

Generally I just report the bugs and forget about them.  If I send a
patch, then I'll review all the warnings from the file and so maybe I'll
see a case where the bug report was overlooked.

I've only seen one case where we merged a fix and then accidentally
merged the buggy code back again the next week.  All the kernel bots had
marked it as old so the bug wasn't caught until later.

Actually that's not true.  Back in the day there used to be giant SCSI
drivers that did their work out of the tree.  Then twice a year they
would do a massive code dump and over write all the fixes that upstream
had merged.  :P  But that never happens now.

regards,
dan carpenter

RE: Smatch mailing list archives

[Reshetova, Elena]

> > On May 10, 2021, at 5:31 AM, Reshetova, Elena <elena.reshetova@intel.com>
> wrote:
> >
> >>> On May 10, 2021, at 2:17 AM, Reshetova, Elena <elena.reshetova@intel.com>
> >> wrote:
> >>>
> >>> What is the best way to create identifiers for the findings that certain smatch
> >>> pattern finds in the kernel? Let's say I have a new pattern that is able to find
> >>> different problematic places and report them in usual smatch way: errors and
> >>> warnings with file name, line number, function name, etc.
> >>> Now for our pattern in order to be sure that the reported issue exists/does not
> >>> exists, somebody needs to go and look at the code manually and make a call.
> >>> After this, it would be nice to mark this place as safe/concern in the report and
> be
> >>> able to transfer these results for kernel versions bumps (5.11->5.12, etc.) as
> soon
> >> as
> >>> the code in this function where finding was reported has not changed (and there
> >>> might be multiple findings per function).
> >>
> >> When I was doing a similar thing to integrate smatch into gerrit, I decided that
> >> line numbers are too unreliable a metric to really depend on.
> >> Instead for most cases just using the rest of the message seems to be good
> enough?
> >> Function name, whatever variable or the message there is and such is used to
> filter
> >> out
> >> known failures (two kinds: false positives and actual bugs that need to be fixed,
> but
> >> are yet unfixed so no point in alerting people to them again and again on
> unrelated
> >> review requests)
> >>
> >> Seems to be working well though I am sure might not be super perfect.
> >
> > I don't think this would be good enough for our case, since what we essentially
> looking
> > for with smatch is a set of function calls to a specific functions. So, I end up with
> > smth like:
> >
> > drivers/vme/boards/vme_vmivme7805.c:69 vmic_probe() warn: read using
> function
> > 'ioread32' to an int type local variable 'data', type is uint
> > drivers/vme/boards/vme_vmivme7805.c:74 vmic_probe() warn: read using
> function
> > 'ioread32' to an int type local variable 'data', type is uint
> >
> > The two above items are in the same function vmic_probe(), making the same
> function
> > call " ioread32" and writing down the result of this function call into the same var
> 'data'.
> > They just happen to do this in two different places in vmic_probe function.
> > So, essentially they are even exactly the same lines in this case and the only
> difference is
> > relative line position within the analyzed vmic_probe() function.
> 
> So is your concern that one of those warnings might be a real bug while another one
> is not? Because if they both are correct or both are incorrect seemingly
> it does not matter (for filtering purposes) that you have two instead of one?
> 

It does matter in our case, because the fact whenever we have a bug or not depends
on what happens to the 'data' variable after (and in this case before the next read into
this variable) and it is not easily scriptable.  

Re: Smatch mailing list archives

[Dan Carpenter]

On Mon, May 10, 2021 at 06:17:27AM +0000, Reshetova, Elena wrote:
> > On Fri, May 07, 2021 at 02:22:37PM +0000, Reshetova, Elena wrote:
> > > Hi,
> > >
> > > I have been working for a while now on a new smatch pattern, but
> > > would really appreciate additional information points such as past
> > > email discussions, etc.
> > >
> > > So I am wondering if there is a way to browse through
> > > the archives of this mailing list in order to try to find the
> > > information I need?
> > 
> > Sorry, I don't think it's archived anywhere.  There isn't a lot of
> > traffic on the list.  About three times a year someone reports that
> > Smatch is crashing for them.
> > 
> > I'm always happy to answer questions if there is any way I can help?
> 
> Thank you Dan! I am pretty new with smatch so that's why I was
> hoping to browse through the existing mails to see if my simple questions
> are already answered, but here is my current issue. 
> 
> What is the best way to create identifiers for the findings that certain smatch
> pattern finds in the kernel? Let's say I have a new pattern that is able to find
> different problematic places and report them in usual smatch way: errors and
> warnings with file name, line number, function name, etc. 
> Now for our pattern in order to be sure that the reported issue exists/does not
> exists, somebody needs to go and look at the code manually and make a call. 
> After this, it would be nice to mark this place as safe/concern in the report and be
> able to transfer these results for kernel versions bumps (5.11->5.12, etc.) as soon as
> the code in this function where finding was reported has not changed (and there
> might be multiple findings per function).
> 
> What is the best way of doing it? 
> I was first thinking of using some simple hash for the reported line (lines around, relative
> position within the reported function),
> but now I think I need also to hash the whole function in addition to the finding itself. 
> 
> Then the logic of transferring the result would be:
> 
> For each finding calculate: 
>  1. finding_line_hash: the hash of the line that resulted in finding (becomes a unique id
>      within the function).
> 2. finding_function_hash: the hash of the function that produced the finding (becomes a
>    unique global id within the kernel) and helps to determine if the function has not been
>    changed between the kernel versions. 
> 
> Logic for the result transfer: 
> 
> If both finding_line_hash and finding_function_hash match between the two smatch reports
> for two different versions, then it is relatively safe to transfer this concrete smatch finding
> and its manual audit result automatically.
> 
> Does it make sense overall? If yes, what is the easiest way in smatch to get hash data for 
> 1 and 2? I.e. get full reported line as a string and full function content as a string? 

I use the a script smatch_scripts/new_bugs.pl  It strips out the
variables names from the single quotes and any numbers and the
parentheses so it looks like this:

Original warning:

    fs/fuse/virtio_fs.c:1468 virtio_fs_get_tree() error: double free of 'fm'

Stripped:

    fs.fuse.virtio_fs.c.virtio_fs_get_tree_error:_double_free_of_''

You could hash the stripped string.  Looking at it now, the variable
name is actually useful and shouldn't be stripped out.  Doh...

I don't know what the zero day bot does for this to mark warnings as
dealt with or not.  There is also the Aiaiai project
(https://www.openhub.net/p/aiaiai) which probably has a feature for
marking warnings as reviewed.

regards,
dan carpenter

RE: Smatch mailing list archives

[Reshetova, Elena]

> On Mon, May 10, 2021 at 06:17:27AM +0000, Reshetova, Elena wrote:
> > > On Fri, May 07, 2021 at 02:22:37PM +0000, Reshetova, Elena wrote:
> > > > Hi,
> > > >
> > > > I have been working for a while now on a new smatch pattern, but
> > > > would really appreciate additional information points such as past
> > > > email discussions, etc.
> > > >
> > > > So I am wondering if there is a way to browse through
> > > > the archives of this mailing list in order to try to find the
> > > > information I need?
> > >
> > > Sorry, I don't think it's archived anywhere.  There isn't a lot of
> > > traffic on the list.  About three times a year someone reports that
> > > Smatch is crashing for them.
> > >
> > > I'm always happy to answer questions if there is any way I can help?
> >
> > Thank you Dan! I am pretty new with smatch so that's why I was
> > hoping to browse through the existing mails to see if my simple questions
> > are already answered, but here is my current issue.
> >
> > What is the best way to create identifiers for the findings that certain smatch
> > pattern finds in the kernel? Let's say I have a new pattern that is able to find
> > different problematic places and report them in usual smatch way: errors and
> > warnings with file name, line number, function name, etc.
> > Now for our pattern in order to be sure that the reported issue exists/does not
> > exists, somebody needs to go and look at the code manually and make a call.
> > After this, it would be nice to mark this place as safe/concern in the report and be
> > able to transfer these results for kernel versions bumps (5.11->5.12, etc.) as soon
> as
> > the code in this function where finding was reported has not changed (and there
> > might be multiple findings per function).
> >
> > What is the best way of doing it?
> > I was first thinking of using some simple hash for the reported line (lines around,
> relative
> > position within the reported function),
> > but now I think I need also to hash the whole function in addition to the finding
> itself.
> >
> > Then the logic of transferring the result would be:
> >
> > For each finding calculate:
> >  1. finding_line_hash: the hash of the line that resulted in finding (becomes a
> unique id
> >      within the function).
> > 2. finding_function_hash: the hash of the function that produced the finding
> (becomes a
> >    unique global id within the kernel) and helps to determine if the function has not
> been
> >    changed between the kernel versions.
> >
> > Logic for the result transfer:
> >
> > If both finding_line_hash and finding_function_hash match between the two
> smatch reports
> > for two different versions, then it is relatively safe to transfer this concrete smatch
> finding
> > and its manual audit result automatically.
> >
> > Does it make sense overall? If yes, what is the easiest way in smatch to get hash
> data for
> > 1 and 2? I.e. get full reported line as a string and full function content as a string?
> 
> I use the a script smatch_scripts/new_bugs.pl  It strips out the
> variables names from the single quotes and any numbers and the
> parentheses so it looks like this:
> 
> Original warning:
> 
>     fs/fuse/virtio_fs.c:1468 virtio_fs_get_tree() error: double free of 'fm'
> 
> Stripped:
> 
>     fs.fuse.virtio_fs.c.virtio_fs_get_tree_error:_double_free_of_''
> 
> You could hash the stripped string.  Looking at it now, the variable
> name is actually useful and shouldn't be stripped out.  Doh...

Even with the variable name would not be enough for us. 
The below two lines would hash to the same value:

drivers/vme/boards/vme_vmivme7805.c:69 vmic_probe() warn: read using function
 'ioread32' to an int type local variable 'data', type is uint
drivers/vme/boards/vme_vmivme7805.c:74 vmic_probe() warn: read using function
 'ioread32' to an int type local variable 'data', type is uint

So, I really need a relative position within the function at least to
distinguish between the two cases above. I can do it with scripting I guess on
top, but would be handy if I can get the line number where vmic_probe()
is defined from within the smatch pattern and calculate relative position when
printing the smatch error/warning. 

> 
> I don't know what the zero day bot does for this to mark warnings as
> dealt with or not.  There is also the Aiaiai project
> (https://www.openhub.net/p/aiaiai) which probably has a feature for
> marking warnings as reviewed.

Thank you very much for pointers, let me see if I can use smth from them!

Best Regards,
Elena.


> 
> regards,
> dan carpenter