Messages in this thread | | | Date | Tue, 27 Jul 2021 14:38:47 +0200 | From | Robert Richter <> | Subject | Re: [PATCH] drivers/edac/edac_mc: Remove all strcpy() uses |
| |
On 25.07.21 18:29:54, 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(). > > However, to add labels is better to use the scnprintf to simplify the > arithmetic. > > This is a previous step in the path to remove the strcpy() function > entirely from the kernel. > > Signed-off-by: Len Baker <len.baker@gmx.com> > --- > This is a task of the KSPP [1] > > [1] https://github.com/KSPP/linux/issues/88 > > drivers/edac/edac_mc.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/drivers/edac/edac_mc.c b/drivers/edac/edac_mc.c > index f6d462d0be2d..1286364f0e48 100644 > --- a/drivers/edac/edac_mc.c > +++ b/drivers/edac/edac_mc.c > @@ -1027,6 +1027,7 @@ void edac_mc_handle_error(const enum hw_event_mc_err_type type, > { > struct dimm_info *dimm; > char *p; > + size_t p_size = 0;
I would rather use a 'left' variable which is initialized with sizeof(e->label) close to there p = e->label is.
> int row = -1, chan = -1; > int pos[EDAC_MAX_LAYERS] = { top_layer, mid_layer, low_layer }; > int i, n_labels = 0; > @@ -1113,12 +1114,11 @@ void edac_mc_handle_error(const enum hw_event_mc_err_type type, > p = e->label; > *p = '\0'; > } else { > - if (p != e->label) { > - strcpy(p, OTHER_LABEL); > - p += strlen(OTHER_LABEL); > - } > - strcpy(p, dimm->label); > - p += strlen(p); > + const char *or = (p != e->label) ? OTHER_LABEL : ""; > + > + p_size += scnprintf(p + p_size, > + sizeof(e->label) - p_size, > + "%s%s", or, dimm->label);
My preference is to advance p here (and decrement 'left'). This is the pattern how p is used throughout the code. I also don't see a benefit of using scnprintf() here compared to the previous implementation.
Thanks,
-Robert
> } > > /* > @@ -1140,9 +1140,9 @@ void edac_mc_handle_error(const enum hw_event_mc_err_type type, > } > > if (any_memory) > - strcpy(e->label, "any memory"); > + strscpy(e->label, "any memory", sizeof(e->label)); > else if (!*e->label) > - strcpy(e->label, "unknown memory"); > + strscpy(e->label, "unknown memory", sizeof(e->label)); > > edac_inc_csrow(e, row, chan); > > -- > 2.25.1 >
| |