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

Greg Kroah-Hartman
2024-05-22T03:57:40Z

On Tue, May 21, 2024 at 07:56:54PM +0200, Michal Hocko wrote:
> On Tue 21-05-24 19:03:58, Greg KH wrote:
> > On Tue, May 21, 2024 at 06:51:28PM +0200, Michal Hocko wrote:
> [...]
> > And really, in the end, if you have a "CVE and fix for CVE" in the same
> > release, applying both doesn't hurt anyone, so this is a "fail secure"
> > mode for everyone, right?
>
> Look Greg, we have been through this discussion at several occasions and
> I do not want to repeat that again. Stable tree users probably do not
> care because they are getting all these patches, including regressions
> and patches they do not need or even want, anyway. They are getting what
> they are _paying_ for. Marking them CVE doesn't make any difference. But
> stable tree backporting model is simply not a good fit for _many_ users.

I understand you feel this way, but I can still disagree that it results
in a secure system for users. But let's ignore that for now...

> Now, for $reasons, people _do_ care about CVEs and that is why there is
> an engineering cost on downstreams to review them. Exactly because
> applying all of them blindly is a _risk_. Exactly because the stable
> backporting model/policy and CVE assigning policy is simply incompatible
> with the stability/correctness/performance/$other requirements.

That's your business decision, NOT mine.

> 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? Both issues involved here
classify as such, the only issue is that the fix/revert showed up in the
same release. And yes, as such, it does not qualify under the CVE rules
as they want to see the bug in a release, and so our tools normally
catch this type of thing.

BUT if you have a model of "I cherry-pick what I want", you WILL miss
things like this that could be considered serious. So note that by us
not classifying stuff like this as a CVE, YOU run the risk of missing
this and maybe ending up with a bug in your system (i.e. you took the
bug, but not the fix.)

That's your risk, yes, and your business decision to do so, the commits
here in question being marked as a CVE make your life easier, not
harder, so while I will go revoke these, realize this means that you now
need to do more work, not less, for the future. Sorry about that.

> 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. Personally I have "wasted" weeks, if not months, of development
time I could have spent in review of kernel patches, or writing kernel
fixes, in order to get this all working properly, having meetings, going
through "CVE CNA training", writing bash scripts, writing tests for bash
scripts, fixing bash scripts based on horrible data in free-form
changelog commits, and most importantly, reviewing 100+ commits a week
yet-again.

Then there is the other developers here doing this work, it's not just
me. For every CVE you see assigned, it has been reviewed and agreed
apon by at LEAST 2 different independent people. Usually 3. That is
anything but "unattended". If you wanted "unattended", I would just
crank out a CVE for every stable commit that is added to the tree, but
that's not what is happening. We are manually reviewing every commit to
see if it matches up with the CVE vulnerability guidelines and
classifying it as such. To tell people that the work they do is not
even happening is rude.

Also, this is work ostensibly that you are also already doing for your
day-job, right? Surely you also are reviewing each and every commit
that ends up in the stable kernels at the very least to evaluate if they
are security or just bug fixes for your corporate offerings, right? So
you already have this work done, why does it matter if a CVE is assigned
to a commit or not, you all already know if it is relevent or not for
your kernels before that assignment happens, so you can trivially just
match up the ids to see if perhaps you missed something or not, right?

So along those lines, why not help us out and provide us with a list of
commits that you feel _should_ be assigned CVEs, and an annotated list
of those that you feel _should not_ be assigned, like we are currently
doing today as part of our review process? We already have external
people helping us out already, sending us patches for our review lists
and providing a fourth and fifth set of review eyes on things for where
we miss stuff, OR where we get things wrong (hey, we are human, we will
do both as this is NOT automated.)

I appreciate your reviews so far of "hey, this shouldn't really be a
CVE, right?" That's a lot of help and is making things better, but to
insinuate that somehow we don't know what the hell we are doing, or that
we are doing nothing other than a simple "assign everything!" process
here is extremely insulting to me and to the other developers here
working on this.

So again, I welcome your help, and I understand your frustration that
somehow you feel we are now making you do more work than before, but if
that really is the case, then you really weren't actually looking at the
patch stream of fixes that you should have been looking at, and I doubt
that's something you want to admit to in public :)

thanks,

greg k-h