From: Andrew Morton <akpm@linux-foundation.org>
To: Denis Kirjanov <kirjanov@gmail.com>
Cc: linux-edac@vger.kernel.org, linux-kernel@vger.kernel.org,
stable@vger.kernel.org, Doug Thompson <dougthompson@xmission.com>,
Borislav Petkov <borislav.petkov@amd.com>
Subject: Re: [PATCH] edac: fix buffer overrun if no suitable bandwidth found
Date: Tue, 23 Oct 2012 12:10:05 -0700 [thread overview]
Message-ID: <20121023121005.ed33061a.akpm@linux-foundation.org> (raw)
In-Reply-To: <1350919858-26202-1-git-send-email-kirjanov@gmail.com>
On Mon, 22 Oct 2012 19:30:58 +0400
Denis Kirjanov <kirjanov@gmail.com> wrote:
> fix buffer overrun if no suitable bandwidth found
>
> Signed-off-by: Denis Kirjanov <kirjanov@gmail.com>
> ---
> drivers/edac/amd64_edac.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
> index 5a297a2..d85ad9e 100644
> --- a/drivers/edac/amd64_edac.c
> +++ b/drivers/edac/amd64_edac.c
> @@ -188,6 +188,9 @@ static int __amd64_set_scrub_rate(struct pci_dev *ctl, u32 new_bw, u32 min_rate)
> * scrubrates array.
> */
> }
> + if (i == ARRAY_SIZE(scrubrates)) {
> + i--;
> + }
>
> scrubval = scrubrates[i].scrubval;
>
That's pretty strange code in there.
If the comment is to be believed, isn't this a suitable fix?
--- a/drivers/edac/amd64_edac.c~a
+++ a/drivers/edac/amd64_edac.c
@@ -171,7 +171,7 @@ static int __amd64_set_scrub_rate(struct
* bandwidth entry that is greater or equal than the setting requested
* and program that. If at last entry, turn off DRAM scrubbing.
*/
- for (i = 0; i < ARRAY_SIZE(scrubrates); i++) {
+ for (i = 0; i < ARRAY_SIZE(scrubrates) - 1; i++) {
/*
* skip scrub rates which aren't recommended
* (see F10 BKDG, F3x58)
_
Also, I don't think "buffer overrun" is an appropriate description here
- to me, "buffer overrun" implies writing to memory outside the buffer.
I'd call this "array overindexing" or similar.
Finally, when fixing a bug, please always describe the user-visible
impact of that bug. You have cc'ed stable on this patch (using the
incorrect email address, btw) which implies that the effects are serious,
but people will want to know specific details about those effects when
considering the patch.
next prev parent reply other threads:[~2012-10-23 19:10 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-10-22 15:30 [PATCH] edac: fix buffer overrun if no suitable bandwidth found Denis Kirjanov
2012-10-23 15:39 ` Borislav Petkov
2012-10-23 19:10 ` Andrew Morton [this message]
2012-10-23 20:26 ` Borislav Petkov
2012-10-23 20:37 ` Andrew Morton
2012-10-23 20:49 ` Borislav Petkov
2012-10-23 21:09 ` Andrew Morton
2012-10-23 21:38 ` Borislav Petkov
2012-10-23 21:58 ` Andrew Morton
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20121023121005.ed33061a.akpm@linux-foundation.org \
--to=akpm@linux-foundation.org \
--cc=borislav.petkov@amd.com \
--cc=dougthompson@xmission.com \
--cc=kirjanov@gmail.com \
--cc=linux-edac@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox