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-next>] [day] [month] [year] [list]
Message-ID: <SJ1PR11MB60839A0FC6B5E79E3E5A7997FC29A@SJ1PR11MB6083.namprd11.prod.outlook.com>
Date:   Mon, 3 Jul 2023 21:51:33 +0000
From:   "Luck, Tony" <tony.luck@...el.com>
To:     Markus Elfring <Markus.Elfring@....de>,
        Koba Ko <koba.ko@...onical.com>,
        "linux-edac@...r.kernel.org" <linux-edac@...r.kernel.org>,
        "kernel-janitors@...r.kernel.org" <kernel-janitors@...r.kernel.org>,
        "Borislav Petkov" <bp@...en8.de>,
        James Morse <james.morse@....com>,
        "Mauro Carvalho Chehab" <mchehab@...nel.org>,
        Robert Richter <rric@...nel.org>
CC:     LKML <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v2] EDAC/i10nm: shift exponent is negative

> > UBSAN complains this error
> > ~~~
> >  UBSAN: shift-out-of-bounds in drivers/edac/skx_common.c:369:16
> …
> > ~~~
> >
> > when get rows, cols and ranks, the returned error value doesn't be
> > handled.
> >
> > check the return value is EINVAL, if yes, directly return 0.
> …
>
> * Please improve this change description further.

To be specific. Initially you reported this because of the UBSAN error
report. But, after community discussion you now know that the problem
is that one or more of the rows/cols/ranks has a value that the EDAC driver
doesn't expect and probably can handle.

So, in V2, the commit message should start with the information these
values are out of range and mention this was discovered when UBSAN
put out a warning about a negative shift. No need to include the whole
of the UBSAN stack trace.

Then describe the two fixes that this patch includes. One is to change the
edac debug message into a console error message to enable further
debug of this issue. The other is to skip the unrecognized DIMM.

> * How do you think about to add the tag “Fixes”?

This is a good idea.  Use git blame, or dig into the GIT history to
find the commit where this code was introduced (hint .. git blame
says:
88a242c98740 ("EDAC, skx_common: Separate common code out from skx_edac")
but that obviously just refactored code, so you should dig back more into
the history.

> > V2: make error-print explicitly
> > ---
>
> Would you like to avoid a misplaced marker line here?
>
> See also:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.4#n686

That's an excellent resource.

-Tony

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ