Re: CVE-2024-35906: drm/amd/display: Send DTBCLK disable message on first commit

[Greg Kroah-Hartman]

On Thu, May 23, 2024 at 10:26:56AM +0200, Michal Hocko wrote:
> On Wed 22-05-24 05:57:38, Greg KH wrote:
> [...]
> > > I completely do get why you do not care about that downstream
> > > engineering cost but generating bogus CVEs on top of a huge pile of
> > > dubious ones is less than useful, don't you think?
> > 
> > How is this a "bogus" CVE on their own?
> 
> I suspect you haven't looked at those commits. This is a boot time
> suboptimal HW configuration. There is no way any user can exploit that I
> can see. Not to mention it cases system boot hangs!

Yes, you are correct, the original should not have had a CVE assigned to
it, that was wrong, and I have rejected it now.  Same for the revert, it
too is now rejected.  Thanks for the review, it is much appreciated.

Also, the reason the original had a cve assigned to it was a fault on my
side, that shouldn't have happened, and I've re-reviewed to make sure
that I didn't mark anything else that way as well (so far I have not
found anything, the 'revert' caused problems in our tools, not to blame
them, but me, the author of that tool...)

> [...]
> > > Seriously, we can disagree whether something is a security threat that
> > > is worth marking by a CVE. But making the CVE generation process mostly
> > > unattended script driven process without any _serious_ review in place
> > > is burning a lot of man power that could be used in a much more
> > > productive way. This is not the way how to convince people to use stable
> > > kernels.
> > 
> > To think that any of this is an "unattended script without any _serious_
> > review" is slandering all of the people who put in their free time to do
> > this work for you and the community.  This is ANYTHING BUT an unattended
> > script.
> 
> This is a perception one can easily draw by watching the stream of
> incoming flow. Sorry if that is not the case but there is about zero
> transparency about the process except for Documentation/process/cve.rst
> and let me quote
> "
> As part of the normal stable release process, kernel changes that are
> potentially security issues are identified by the developers responsible
> for CVE number assignments and have CVE numbers automatically assigned
> to them.
> "
> 
> There is no mention about criteria, review process. Who is involve in
> the assignment and who is doing the review. The vulns.git tree doesn't
> contain Sign-off-by of those involved parties except for the submitter.

The "criteria" is that as described by cve.org, we can't do anything
about that.  The process, yes, we can be more open about that but as it
has been evolving over time, it's hard to describe a moving target at
times.  I know Lee is writing up an article about how this all works,
and I'm going to be giving talks at conferences in a few months as well.
And people also just ask us directly, which you can :)

Because of asking, many others are starting to help out, you can too,
just submit patches against the cve/review/proposed/ directory with a
list of commits that you feel should have CVEs assigned for, or annotate
why you feel specific ones we have reviewed should NOT have a CVE
assigned, and our tools can handle them quite well as part of the
assignment process (see scripts/cve_review for a tool that some of us
use to create these files, that's not required, as not all of us use it,
but the output format is the key, and that's a simple list of commit
ids, personally I generate that from mboxes.)

> > Also, this is work ostensibly that you are also already doing for your
> > day-job, right?
> 
> We, like stable trees, rely on Fixes tag and those (including other
> commits that might be this tag) are reviewed by domain experts.

Great, so you already have reviewed all of these, so it should be a
simple match up on your end.

> I have raised a concern based on observed CVE flow that the current
> process is automated without a proper review as I can see very dubious
> CVEs being assigned (splats resembling oops/warnings coming from lockdep,
> data_race fixes as they resemble race condition fixes, READ_ONCE fixes
> which do not fix anything discussed in other thread and others).

It's a learning process for all of us involved, and I thank you for your
reviews to help make this better.

> I have tried to dispute quite some but quickly learned that many of them
> have been dismissed because "no usecases are assumed". It is a very
> broad category that could indeed make any fix a CVE.

We can't assume usecases, sorry, you know that.  And yes, it does make
it such that many broad categories can get a CVE, which is just part of
the "business" when working at this low level in the stack.

> If you really want to build a trust around the CVE process then make it
> more transparent. I would start by adding reason why something has been
> marked CVE. You are saying there is peer review process going on then
> why not add Reviewed-by: to make it explicit and visible.

We have that, see the git log of the cve/review/ directory for the files
and where most all of the cves come from.  Some come directly from
requests by others to us, and a few other places (i.e. security
reports), so we of course can't document the source of everything for
obvious reasons.  But always feel free to ask if you think something
looks "odd" and we'll do the best that we can to answer it.

thanks,

greg k-h