On Fri, Jul 26, 2024 at 09:17:20AM +0100, Tvrtko Ursulin wrote: > > On 25/07/2024 16:58, Dan Carpenter wrote: > > On Thu, Jul 25, 2024 at 08:48:35AM +0100, Tvrtko Ursulin wrote: > > > > > > Hi, > > > > > > On 12/07/2024 22:41, Jonathan Cavitt wrote: > > > > Prevent a NULL pointer access in intel_memory_regions_hw_probe. > > > > > > For future reference please include some impact assessment in patches tagged > > > as fixes. Makes maintainers job, and even anyone's who tries to backport > > > stuff to stable at some future date, much easier if it is known how > > > important is the fix and in what circumstances can the problem it is fixing > > > trigger. > > > > > > > As someone doing backport work, I think this patch is fine. Everyone > > knows the impact of a NULL dereference in probe(). > > > > I guess with patches that add NULL dereferences, the trick is > > understanding when people are adding NULL checks to make a static > > checker happy or when it's a real bug. But the fault lies with the > > people adding NULL checks just to make the tools happy. Some of these > > pointless NULL checks end up in stable, but it's fine, extra NULL checks > > never hurt anyone. If the maintainer wants to be extra safe by adding > > NULL checks then who are we to say otherwise. > > > > In other words normal patches shouldn't have to say. "I'm not lying" at > > the end. It should be the pointless patches which say, "I'm doing a > > pointless thing. Don't bother backporting." > > > > Most stable patch backports are done automatically and people have > > various tools and scripts to do that. If the tools don't handle this > > patch automatically then they are defective. > > Right, and every few releases maintainers and authors get a bunch of emails > for patches which did not apply to some stable tree. > I believe these emails are only sent for commits that are tagged for stable. For AUTOSEL patches, the backporting is done on a best effort basis. On the other hand, hopefully this patch would have been tagged for stable if we hadn't fixed the bug so quickly. > In which case someone has to do manual work and then it is good to know how > important it is to backport something. For cases when it is not trivial. It > does not apply to this patch, but as a _best practice_ it is good if the > commit message explains the impacted platforms and scenarios. > > In this case I can follow the Fixes: tag and see the fix that this patches > fixes is only about ATS-M. Which if it was a more complicated patch might be > a reason to not need bother backporting past some kernel version where > platform X wasn't even supported. > > Therefore I think my point is that best practice is to include this the > commit text, so any future maintainer/backporter does not have to re-do > detective work, stands. This is a really elaborate hypothetical. Are there kernels which are affected by this bug but don't support ATS-M? regards, dan carpenter