Re: New CVE entries this week

[Masami Ichikawa <masami.ichikawa@miraclelinux.com>]

Hi.

On Thu, Jan 19, 2023 at 4:51 PM Dan Carpenter <error27@gmail.com> wrote:
>
> On Thu, Dec 15, 2022 at 12:25:18PM +0900, Masami Ichikawa wrote:
> > CVE-2022-4378: Linux kernel stack-based buffer overflow in __do_proc_dointvec
> >
> > CVSS v3 score is not provided
> >
> > A stack overflow bug was found in __do_proc_dointvec() which missed
> > checking on user input.
> > This bug affected all stable kernels. It seems as if 4.4 is affected too.
> >
> > Fixed status
> > mainline: [bce9332220bd677d83b19d21502776ad555a0e73,
> > e6cfaf34be9fcd1a8285a294e18986bfc41a409c]
>
> One thing that we used to do at Oracle was a bi-weekly meeting where we
> would go through these lists and try to be a bit proactive about
> preventing future bugs.  For me I'm trying to use Smatch for static
> analysis.
>
> There are some bugs which Smatch can't identify like race conditions or
> if there is an issue with the spec.  But for a lot of bugs can be
> prevented.  So it's often an issue of 1) There isn't a Smatch check for
> that.  2) The Smatch check exists but isn't working correctly.  3) The
> Smatch check prints a warning but there are too many warning for that
> check so I can't go through them all.
>
> First of all, why wasn't *size marked as user controlled?  It turned out
> that it comes from iov_iter_count() and that wasn't marked as user
> controlled.  Fix that:
> https://github.com/error27/smatch/commit/70ee7aa1ae8cc07767096e16fa2de68a62507a3e
>
> Once that was fixed, it turned out that I did have an unpublished check
> which printed a warning.
> kernel/sysctl.c:358 proc_get_long() warn: check 'tmp[len]' for negative offsets 'len' = s32min.  extra = 's32min-21'
>
> But it turns out that warning was because of a bug.  The check was
> asking can "*size" be user controlled and what is the minimum possible
> value negative, but it should have been asking if the minimum user
> controled value is negative.
>
> Fixing the check to as about user controlled values silenced the
> warning.  The issue with that is:
>
>         left -= proc_skip_spaces(&p);
>
> Subtractions are very hard to handle correctly because you need to keep
> track of the relationships between multiple variables.  Smatch
> deliberately assumes that this subtraction cannot underflow.  Otherwise
> you end up with too many false positives...
>
> I've been sitting on this check for the past ten years without
> publishing it.  May as well attach it now and also the results.  I don't
> know why the check has __per_cpu_offset stuff or why it ignores ntohl().
> I should probably delete that and see what happens.  Going through the
> results, a bunch of false positives are cause by subtraction (which is
> complicated).  Or because Smatch doesn't understand about
> array_index_nospec() (I should fix that).
>
> Anyway, even though I wasn't able to generate a warning for this bug,
> it was still useful to have the discussion and improve Smatch.
>

Thank you for the information about Smatch. It's really helpful. I
think it is important to learn from reported bugs then prevent future
bugs as you did.
I'll try to use Smatch.

> regards,
> dan carpenter
>

Regards,
-- 
Masami Ichikawa
Cybertrust Japan Co., Ltd.

Email :masami.ichikawa@cybertrust.co.jp
          :masami.ichikawa@miraclelinux.com