On Tue, Oct 23, 2012 at 02:09:39PM -0700, Andrew Morton wrote: > On Tue, 23 Oct 2012 22:49:26 +0200 > Borislav Petkov <b...@amd64.org> wrote: > > > Now you only need to give me a proper patch with your S-O-B and we're > > ready to go :). > > who, me, what?!?! Sounds stressful.
Yeah, this is to show you how we all feel when we're on the sending end of patches :-). > > umm, OK here we go. > > What I don't understand is the effects of the bug. If the present code > can indeed start using the n+1th element of the array then it's writing > random garbage into the hardware. If this can ever happen then I > suspect that yes, a cc:stable is needed? Yes, cc:stable is needed. The code has a very subtle bug because I should've robustified it from the get-go instead of fiddling with it. Basically, this is what happens: If you supply new_bw < 761, i.e. so that the last element matches, *and* you have a minimum scrubrate set to != 0x0 (due to unsupported scrubrate settings on some families) the first continue in the loop with i being 22 (last array element) terminates the loop after incrementing i one last time. Thus, it doesn't let us break out of it in the next statement where the new_bw will match and we'll be able to exit the loop without it incrementing i one last time and thus get us out of bounds. So yes, your fix is correct, I'll test it tomorrow just in case and will sit down and rewrite this loop because it pisses me off to see how stupidly fragile it is. And I see you'll carry this patch so I won't send it to Linus next merge window so thanks for this! > From: Andrew Morton <a...@linux-foundation.org> > Subject: drivers/edac/amd64_edac.c:__amd64_set_scrub_rate(): avoid > overindexing scrubrates[] > > If none of the elements in scrubrates[] matches, this loop will cause > __amd64_set_scrub_rate() to incorrectly use the n+1th element. > > As the function is designed to use the final scrubrates[] element in the > case of no match, we can fix this bug by simply terminating the array > search at the n-1th element. > > Reported-by: Denis Kirjanov <kirja...@gmail.com> > Cc: Doug Thompson <dougthomp...@xmission.com> > Cc: Borislav Petkov <borislav.pet...@amd.com> Acked-by: Borislav Petkov <borislav.pet...@amd.com> Cc: sta...@vger.kernel.org > Signed-off-by: Andrew Morton <a...@linux-foundation.org> [ … ] -- Regards/Gruss, Boris. Advanced Micro Devices GmbH Einsteinring 24, 85609 Dornach GM: Alberto Bozzo Reg: Dornach, Landkreis Muenchen HRB Nr. 43632 WEEE Registernr: 129 19551 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
