* Re: CVE-2024-35906: drm/amd/display: Send DTBCLK disable message on first commit [not found] <2024051954-CVE-2024-35906-1c6f@gregkh> @ 2024-05-21 8:28 ` Michal Hocko 2024-05-21 14:39 ` Greg Kroah-Hartman 0 siblings, 1 reply; 13+ messages in thread From: Michal Hocko @ 2024-05-21 8:28 UTC (permalink / raw) To: cve, linux-kernel; +Cc: linux-cve-announce, Greg Kroah-Hartman CVE-2024-35881 to revert f341055b10bd ("drm/amd/display: Send DTBCLK disable message on first commit") by 3a6a32b31a11 ("Revert "drm/amd/display: Send DTBCLK disable message on first commit"") has been filed as well. Is this really intentional? Should both be rejected? On Sun 19-05-24 10:35:20, Greg KH wrote: > Description > =========== > > In the Linux kernel, the following vulnerability has been resolved: > > drm/amd/display: Send DTBCLK disable message on first commit > > [Why] > Previous patch to allow DTBCLK disable didn't address boot case. Driver > thinks DTBCLK is disabled by default, so we don't send disable message to > PMFW. DTBCLK is then enabled at idle desktop on boot, burning power. > > [How] > Set dtbclk_en to true on boot so that disable message is sent during first > commit. > > The Linux kernel CVE team has assigned CVE-2024-35906 to this issue. > > > Affected and fixed versions > =========================== > > Fixed in 6.8.5 with commit 0dab75b433ed > Fixed in 6.9 with commit f341055b10bd > > Please see https://www.kernel.org for a full list of currently supported > kernel versions by the kernel community. > > Unaffected versions might change over time as fixes are backported to > older supported kernel versions. The official CVE entry at > https://cve.org/CVERecord/?id=CVE-2024-35906 > will be updated if fixes are backported, please check that for the most > up to date information about this issue. > > > Affected files > ============== > > The file(s) affected by this issue are: > drivers/gpu/drm/amd/display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c > > > Mitigation > ========== > > The Linux kernel CVE team recommends that you update to the latest > stable kernel version for this, and many other bugfixes. Individual > changes are never tested alone, but rather are part of a larger kernel > release. Cherry-picking individual commits is not recommended or > supported by the Linux kernel community at all. If however, updating to > the latest release is impossible, the individual changes to resolve this > issue can be found at these commits: > https://git.kernel.org/stable/c/0dab75b433ed2480d57ae4f8f725186a46223e42 > https://git.kernel.org/stable/c/f341055b10bd8be55c3c995dff5f770b236b8ca9 -- Michal Hocko SUSE Labs ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: CVE-2024-35906: drm/amd/display: Send DTBCLK disable message on first commit 2024-05-21 8:28 ` CVE-2024-35906: drm/amd/display: Send DTBCLK disable message on first commit Michal Hocko @ 2024-05-21 14:39 ` Greg Kroah-Hartman 2024-05-21 16:51 ` Michal Hocko 0 siblings, 1 reply; 13+ messages in thread From: Greg Kroah-Hartman @ 2024-05-21 14:39 UTC (permalink / raw) To: Michal Hocko; +Cc: cve, linux-kernel, linux-cve-announce On Tue, May 21, 2024 at 10:28:41AM +0200, Michal Hocko wrote: > CVE-2024-35881 to revert f341055b10bd ("drm/amd/display: Send DTBCLK > disable message on first commit") by 3a6a32b31a11 ("Revert > "drm/amd/display: Send DTBCLK disable message on first commit"") has > been filed as well. > > Is this really intentional? Should both be rejected? I don't think so as we had releases with the original commit in it, which was buggy so then the second one was needed. So if you only took the first fix, you have a problem, and need the second one. If you take both, all is good. If you took neither, also all is good. So be safe and take both :) thanks, greg k-h ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: CVE-2024-35906: drm/amd/display: Send DTBCLK disable message on first commit 2024-05-21 14:39 ` Greg Kroah-Hartman @ 2024-05-21 16:51 ` Michal Hocko 2024-05-21 17:03 ` Greg Kroah-Hartman 0 siblings, 1 reply; 13+ messages in thread From: Michal Hocko @ 2024-05-21 16:51 UTC (permalink / raw) To: Greg Kroah-Hartman; +Cc: cve, linux-kernel, linux-cve-announce On Tue 21-05-24 16:39:51, Greg KH wrote: > On Tue, May 21, 2024 at 10:28:41AM +0200, Michal Hocko wrote: > > CVE-2024-35881 to revert f341055b10bd ("drm/amd/display: Send DTBCLK > > disable message on first commit") by 3a6a32b31a11 ("Revert > > "drm/amd/display: Send DTBCLK disable message on first commit"") has > > been filed as well. > > > > Is this really intentional? Should both be rejected? > > I don't think so as we had releases with the original commit in it, I do not think so. Looking at stable kernel branches: $ git describe-ver 0dab75b433ed2480d57ae4f8f725186a46223e42 v6.8.5~88 $ git describe-ver d6d5622f64f3e07620683d61c880f57965fe1b48 v6.8.5~239 Both of them were released in 6.9-rc1 in Linus tree. I do not see them in any other stable trees. Neither of them is even marked for stable and they seemed to be merged only because of (stable tree) 7ea8a0e12088eb0c which has Stable-dep-of: f341055b10bd ("drm/amd/display: Send DTBCLK disable message on first commit"). Btw note that 7ea8a0e12088eb0c is not marked for stable, nor I see anybody requesting that on lore. Stable rulez! Let's put aside whether f341055b10bd should get a CVE, we have clearly a different view on that but looking at the vulns.git tree both CVEs have been assigned together $ git log ./2024/CVE-2024-35906.sha1 ./2024/CVE-2024-35881.sha1 commit a6191f0053349c3234f690316d6511e97927f28f Author: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Date: Sun May 19 10:35:32 2024 +0200 some 6.8.5 cves assigned Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> which to me indicates that both CVEs were assigned by a script without a proper review which is really unfortunate. Please keep in mind that there are actual consumers of these CVEs and you are burning their time evaluating these noops. A waste of time, if you ask me, and not something that could be just neglected considering how many CVEs you are producing. -- Michal Hocko SUSE Labs ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: CVE-2024-35906: drm/amd/display: Send DTBCLK disable message on first commit 2024-05-21 16:51 ` Michal Hocko @ 2024-05-21 17:03 ` Greg Kroah-Hartman 2024-05-21 17:56 ` Michal Hocko 0 siblings, 1 reply; 13+ messages in thread From: Greg Kroah-Hartman @ 2024-05-21 17:03 UTC (permalink / raw) To: Michal Hocko; +Cc: cve, linux-kernel, linux-cve-announce On Tue, May 21, 2024 at 06:51:28PM +0200, Michal Hocko wrote: > On Tue 21-05-24 16:39:51, Greg KH wrote: > > On Tue, May 21, 2024 at 10:28:41AM +0200, Michal Hocko wrote: > > > CVE-2024-35881 to revert f341055b10bd ("drm/amd/display: Send DTBCLK > > > disable message on first commit") by 3a6a32b31a11 ("Revert > > > "drm/amd/display: Send DTBCLK disable message on first commit"") has > > > been filed as well. > > > > > > Is this really intentional? Should both be rejected? > > > > I don't think so as we had releases with the original commit in it, > > I do not think so. Looking at stable kernel branches: > $ git describe-ver 0dab75b433ed2480d57ae4f8f725186a46223e42 > v6.8.5~88 > $ git describe-ver d6d5622f64f3e07620683d61c880f57965fe1b48 > v6.8.5~239 Ah, missed that, sorry, was thinking about a different set of reverts recently assigned. > Both of them were released in 6.9-rc1 in Linus tree. I do not see them > in any other stable trees. Neither of them is even marked for stable and > they seemed to be merged only because of (stable tree) 7ea8a0e12088eb0c > which has Stable-dep-of: f341055b10bd ("drm/amd/display: Send DTBCLK > disable message on first commit"). Btw note that 7ea8a0e12088eb0c is not > marked for stable, nor I see anybody requesting that on lore. > Stable rulez! > > Let's put aside whether f341055b10bd should get a CVE, we have clearly a > different view on that but looking at the vulns.git tree both CVEs have > been assigned together > $ git log ./2024/CVE-2024-35906.sha1 ./2024/CVE-2024-35881.sha1 > commit a6191f0053349c3234f690316d6511e97927f28f > Author: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Date: Sun May 19 10:35:32 2024 +0200 > > some 6.8.5 cves assigned > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > which to me indicates that both CVEs were assigned by a script > without a proper review which is really unfortunate. Note, we have checks for many of these things, where we have an affected kernel and a fix in the same release, and we do NOT assign a CVE for those types of things. This is the first time that I know of where we have had this happen where the bug and revert was in the same release, hard part here was that the revert didn't have a Fixes: tag, if it did, we would have caught it. > Please keep in mind that there are actual consumers of these CVEs and > you are burning their time evaluating these noops. A waste of time, if > you ask me, and not something that could be just neglected considering > how many CVEs you are producing. We will have a small % of issues that we miss and mess up like this, that's just because we are all human. Your help in reviewing these is greatly appreciated. Right now I still feel we are not catching all that we should be catching to mark as a CVE, so we are open for anyone to help us out with reviews. Luckily we have a few more people helping now as well, which is great. 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? thanks, greg k-h ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: CVE-2024-35906: drm/amd/display: Send DTBCLK disable message on first commit 2024-05-21 17:03 ` Greg Kroah-Hartman @ 2024-05-21 17:56 ` Michal Hocko 2024-05-22 3:57 ` Greg Kroah-Hartman 0 siblings, 1 reply; 13+ messages in thread From: Michal Hocko @ 2024-05-21 17:56 UTC (permalink / raw) To: Greg Kroah-Hartman; +Cc: cve, linux-kernel, linux-cve-announce 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. 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. 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? 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. Bye -- Michal Hocko SUSE Labs ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: CVE-2024-35906: drm/amd/display: Send DTBCLK disable message on first commit 2024-05-21 17:56 ` Michal Hocko @ 2024-05-22 3:57 ` Greg Kroah-Hartman 2024-05-23 8:26 ` Michal Hocko 0 siblings, 1 reply; 13+ messages in thread From: Greg Kroah-Hartman @ 2024-05-22 3:57 UTC (permalink / raw) To: Michal Hocko; +Cc: cve, linux-kernel, linux-cve-announce 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: CVE-2024-35906: drm/amd/display: Send DTBCLK disable message on first commit 2024-05-22 3:57 ` Greg Kroah-Hartman @ 2024-05-23 8:26 ` Michal Hocko 2024-05-23 13:49 ` Greg Kroah-Hartman 0 siblings, 1 reply; 13+ messages in thread From: Michal Hocko @ 2024-05-23 8:26 UTC (permalink / raw) To: Greg Kroah-Hartman; +Cc: cve, linux-kernel, linux-cve-announce 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! [...] > > 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. > 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. [...] > 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. I am sorry if you feel that way but I haven't said anything like that. 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). 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. 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. Thanks! -- Michal Hocko SUSE Labs ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: CVE-2024-35906: drm/amd/display: Send DTBCLK disable message on first commit 2024-05-23 8:26 ` Michal Hocko @ 2024-05-23 13:49 ` Greg Kroah-Hartman 2024-05-24 10:10 ` Michal Hocko 0 siblings, 1 reply; 13+ messages in thread From: Greg Kroah-Hartman @ 2024-05-23 13:49 UTC (permalink / raw) To: Michal Hocko; +Cc: cve, linux-kernel, linux-cve-announce 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: CVE-2024-35906: drm/amd/display: Send DTBCLK disable message on first commit 2024-05-23 13:49 ` Greg Kroah-Hartman @ 2024-05-24 10:10 ` Michal Hocko 2024-05-24 11:47 ` Greg Kroah-Hartman 0 siblings, 1 reply; 13+ messages in thread From: Michal Hocko @ 2024-05-24 10:10 UTC (permalink / raw) To: Greg Kroah-Hartman; +Cc: cve, linux-kernel, linux-cve-announce 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: > > [...] > > > > 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. Thanks for considering the feedback! > 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...) Good to hear this has an explanation. [...] > 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@kernel.org as suggested by the in tree documentation? 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. > > > 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. Not at all. Every incoming CVE has to go through CVSS assessment at least. This on its own is a very non-trivial and time consuming task that obviously scales with the number of CVEs. There is more going on besides that. [...] > > 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. 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). Thanks! -- Michal Hocko SUSE Labs ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: CVE-2024-35906: drm/amd/display: Send DTBCLK disable message on first commit 2024-05-24 10:10 ` Michal Hocko @ 2024-05-24 11:47 ` Greg Kroah-Hartman 2024-05-24 14:02 ` Michal Hocko 0 siblings, 1 reply; 13+ messages in thread From: Greg Kroah-Hartman @ 2024-05-24 11:47 UTC (permalink / raw) To: Michal Hocko; +Cc: cve, linux-kernel, linux-cve-announce 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@kernel.org as suggested by the in tree documentation? The documentation should say that you _SHOULD_ send proposals to cve@kernel.org, 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? > > > 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. 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 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. 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.) Same for "reviewed-by:" we don't want to do that, as again, no personal information can be added to cve records, but as we are putting our reviews in a public repo, you can work backwards if you are curious from there. And again, others are sending us reviews as well, you can see them in the repo already. Hopefully more will do the same as time goes by, and we have a steady stream of developers at various companies sending us requests as well just through email and we handle them that way. We do have one CVE right now that does want the text changed, and we are working on making that possible (by accepting patches to the text) as the changelog text was incorrect and called out an incorrect device as being a problem when it was not. But that's an outlier and we will handle that on a case-by-case basis. thanks, greg k-h ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: CVE-2024-35906: drm/amd/display: Send DTBCLK disable message on first commit 2024-05-24 11:47 ` Greg Kroah-Hartman @ 2024-05-24 14:02 ` Michal Hocko 2024-05-24 15:22 ` Greg Kroah-Hartman 0 siblings, 1 reply; 13+ messages in thread From: Michal Hocko @ 2024-05-24 14:02 UTC (permalink / raw) To: Greg Kroah-Hartman; +Cc: cve, linux-kernel, linux-cve-announce 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@kernel.org as suggested by the in tree documentation? > > The documentation should say that you _SHOULD_ send proposals to > cve@kernel.org, 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: CVE-2024-35906: drm/amd/display: Send DTBCLK disable message on first commit 2024-05-24 14:02 ` Michal Hocko @ 2024-05-24 15:22 ` Greg Kroah-Hartman 2024-05-24 15:59 ` Michal Hocko 0 siblings, 1 reply; 13+ messages in thread From: Greg Kroah-Hartman @ 2024-05-24 15:22 UTC (permalink / raw) To: Michal Hocko; +Cc: cve, linux-kernel, linux-cve-announce On Fri, May 24, 2024 at 04:02:18PM +0200, Michal Hocko wrote: > 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@kernel.org as suggested by the in tree documentation? > > > > The documentation should say that you _SHOULD_ send proposals to > > cve@kernel.org, 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. Yes, you can do that too, send it to same email address. That's not really documented, just ask us instead! :) > > 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. Doing the work to classify all of these in this manner isn't going to happen by us, sorry, as it is not required by the CVE process, and frankly, we are doing a load of work already here. We are going to rely on the text that is in the changelog. Maybe over time you can work with the kernel developers to write better changelogs to describe what you are looking for? We will rely on external parties to "classify" the CVEs if they wish to as there is already a whole ecosystem that attempts to do this already, with various success. In the end, it's up to each integrator of Linux to classify them themselves as everyone's use case is different (remember, cow milking machines, super-mega-yaht-stabilizers, washing machines, servers, watches, air-traffic-control systems, etc...) thanks, greg k-h ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: CVE-2024-35906: drm/amd/display: Send DTBCLK disable message on first commit 2024-05-24 15:22 ` Greg Kroah-Hartman @ 2024-05-24 15:59 ` Michal Hocko 0 siblings, 0 replies; 13+ messages in thread From: Michal Hocko @ 2024-05-24 15:59 UTC (permalink / raw) To: Greg Kroah-Hartman; +Cc: cve, linux-kernel, linux-cve-announce On Fri 24-05-24 17:22:24, Greg KH wrote: [...] > > > 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. > > Doing the work to classify all of these in this manner isn't going to > happen by us, sorry, as it is not required by the CVE process, and > frankly, we are doing a load of work already here. I would really like the understand this position. You are doing the classification already, right? What does prevent you from making that a part of the process? Why wouldn't you like to help CVE consumers to understand that process better? -- Michal Hocko SUSE Labs ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2024-05-24 15:59 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <2024051954-CVE-2024-35906-1c6f@gregkh>
2024-05-21 8:28 ` CVE-2024-35906: drm/amd/display: Send DTBCLK disable message on first commit Michal Hocko
2024-05-21 14:39 ` Greg Kroah-Hartman
2024-05-21 16:51 ` Michal Hocko
2024-05-21 17:03 ` Greg Kroah-Hartman
2024-05-21 17:56 ` Michal Hocko
2024-05-22 3:57 ` Greg Kroah-Hartman
2024-05-23 8:26 ` Michal Hocko
2024-05-23 13:49 ` Greg Kroah-Hartman
2024-05-24 10:10 ` Michal Hocko
2024-05-24 11:47 ` Greg Kroah-Hartman
2024-05-24 14:02 ` Michal Hocko
2024-05-24 15:22 ` Greg Kroah-Hartman
2024-05-24 15:59 ` Michal Hocko
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox