Messages in this thread | | | Date | Mon, 23 Aug 2021 19:30:34 +0200 | From | Borislav Petkov <> | Subject | Re: [PATCH v4] EDAC/mc: Prefer strscpy over strcpy |
| |
On Sat, Aug 14, 2021 at 09:55:27AM +0200, Len Baker wrote: > strcpy() performs no bounds checking on the destination buffer. This > could result in linear overflows beyond the end of the buffer, leading > to all kinds of misbehaviors. The safe replacement is strscpy(). > > This is a previous step in the path to remove the strcpy() function
"previous step"?
> entirely from the kernel. > > Signed-off-by: Len Baker <len.baker@gmx.com>
...
> diff --git a/drivers/edac/edac_mc.c b/drivers/edac/edac_mc.c > index f6d462d0be2d..7aea6c502316 100644 > --- a/drivers/edac/edac_mc.c > +++ b/drivers/edac/edac_mc.c > @@ -1032,6 +1032,7 @@ void edac_mc_handle_error(const enum hw_event_mc_err_type type, > int i, n_labels = 0; > struct edac_raw_error_desc *e = &mci->error_desc; > bool any_memory = true; > + size_t len; > > edac_dbg(3, "MC%d\n", mci->mc_idx); > > @@ -1086,6 +1087,7 @@ void edac_mc_handle_error(const enum hw_event_mc_err_type type, > */ > p = e->label; > *p = '\0'; > + len = sizeof(e->label); > > mci_for_each_dimm(mci, dimm) { > if (top_layer >= 0 && top_layer != dimm->location[0]) > @@ -1114,10 +1116,12 @@ void edac_mc_handle_error(const enum hw_event_mc_err_type type, > *p = '\0'; > } else { > if (p != e->label) { > - strcpy(p, OTHER_LABEL); > - p += strlen(OTHER_LABEL); > + strscpy(p, OTHER_LABEL, len);
Hm, maybe I'm missing something but looking at that strscpy() definition, why aren't you doing:
num = strscpy(p, OTHER_LABEL, len); if (num < 0) /* just in case */ break;
len -= num; p += num;
since that function supposedly returns the number of chars copied.
> + len -= strlen(p); > + p += strlen(p); > } > - strcpy(p, dimm->label); > + strscpy(p, dimm->label, len); > + len -= strlen(p); > p += strlen(p);
Ditto.
Thx.
-- Regards/Gruss, Boris.
https://people.kernel.org/tglx/notes-about-netiquette
| |