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

[Michal Hocko]

On Fri 24-05-24 13:47:00, Greg KH wrote:
> On Fri, May 24, 2024 at 12:10:56PM +0200, Michal Hocko wrote:
> > On Thu 23-05-24 15:49:59, Greg KH wrote:
> > > On Thu, May 23, 2024 at 10:26:56AM +0200, Michal Hocko wrote:
> > > > On Wed 22-05-24 05:57:38, Greg KH wrote:
> > [...]
> > > 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.)
> > 
> > Do I get it right that proposals shouldn't be sent via email to
> > cve@xxxxxxxxxx as suggested by the in tree documentation?
> 
> The documentation should say that you _SHOULD_ send proposals to
> cve@xxxxxxxxxx, did we get it wrong somehow?
> 
> > I do not mind
> > the specific workflow but until now I have followed Documentation/process/cve.rst
> > as authoritative source of the process. It would be really great if that
> > matched the workflow.
> 
> I'm confused as to what in that document is incorrect, care to point it
> out?

Maybe I've just misunderstood the part about sending patches against
cve/review/proposed/. I was thinking about sending pull requests against
vulns.git.
 
> > > > 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.
> > 
> > Thanks for pointing me there but I do not think this is what I've had in
> > mind. I do understand that there is some pattern matching happening to
> > select most of the candidates, but as you've said this is then reviewed
> > and during that review you likely need to read through that changelog
> > and build some sort of statement why this is considered a security
> > threat.
> 
> Right now for the 3 people doing all of the reviews, 1 is using pattern
> matching of a sort (see the cve_review tool for the big regex and
> workflow used there), one is reading each patch/header in mbox format,
> and one is using some other tool.  Then for the ones that we disagree on
> (i.e. not a score of 2 out of 3), we comment as to why we feel they
> should, or should not, be accepted.

Thanks for the clarification.

> As this process is evolving, we
> haven't really documented it, except here and in talks with others as we
> travel.  We're working on making that more public over time.
> 
> Note, I do not know of any other CNA that does this in public as much as
> we are already, we are pushing the boundry of what CNAs are doing pretty
> hard here by putting almost all of our reviews in public _before_ a CVE
> is assigned.  That's not normal, and hopefully we don't get told to stop
> that (sshhh, don't tell anyone...)

I really like and appreciate this part! That is a huge improvement
comparing to the previous process.

> > I believe exactly _this_ would be a much more valuable information in
> > the CVE announcement than a copy&past of the changelog which on its own
> > can be trially referenced by a link. Also if there is a peer review
> > happening then Reviewed-by would be really nice. Not to mention
> > Reported-by if this was externally reported (the report could be a part
> > of the announcement as well).
> 
> We can't do a "Reported-by:" for CVEs as we aren't allowed to add
> personal information like that to the records as per cve.org's rules.

OK, understood. Although I do remember CVEs crediting parties
discovering/reporting the issue.

> And people want to word-smith the text all the time already, so we just
> default to using the changelog text as that's the most "neutral" and
> public information out there (i.e. we don't have to worry about any sort
> of data-retention or classification laws as the information is already
> public in kernel changelog text.)

This part I do not understand. What is wrong about a reasoning why
something has been considered a CVE? E.g. something like 
CVE assigned because a potential WARN_ON is fixed and that could panic
with panic_on_warn. Fixed by <URL_TO_LINUS_TREE>

or
CVE assigned because UAF is fixed and those can be generally used to
construct more complex attacks. Fixed by <URL_TO_LINUS_TREE>

etc.
-- 
Michal Hocko
SUSE Labs