Skip to content

Commit fca6116

Browse files
Len Bakersuryasaimadhu
authored andcommitted
EDAC/mc: Replace strcpy(), sprintf() and snprintf() with strscpy() or scnprintf()
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 misbehavior. The safe replacement is strscpy(). [1][2] However, to simplify and clarify the code, to concatenate labels use the scnprintf() function. This way it is not necessary to check the return value of strscpy() (-E2BIG if the parameter count is 0 or the src was truncated) since scnprintf() always returns the number of chars written into the buffer. This function always returns a nul-terminated string even if it needs to be truncated. While at it, fix all other broken string generation code that wrongly interprets snprintf()'s return code or just uses sprintf(), implement that using scnprintf() here too. Drop breaks in loops around scnprintf() as it is safe now to loop. Moreover, the check is not needed: for the case when the buffer is exhausted, len never gets zero because scnprintf() takes the full buffer length as input parameter, but excludes the trailing '\0' in its return code and thus, 1 is the minimum len. [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strcpy [2] KSPP#88 [ rric: Replace snprintf() with scnprintf(), rework sprintf() user, drop breaks in loops around scnprintf(), introduce 'end' pointer to reduce pointer arithmetic, use prefix pattern for e->location, adjust subject and description ] Co-developed-by: Joe Perches <[email protected]> Signed-off-by: Joe Perches <[email protected]> Signed-off-by: Len Baker <[email protected]> Signed-off-by: Robert Richter <[email protected]> Signed-off-by: Borislav Petkov <[email protected]> Link: https://lkml.kernel.org/r/[email protected]
1 parent 6880fa6 commit fca6116

File tree

1 file changed

+18
-24
lines changed

1 file changed

+18
-24
lines changed

drivers/edac/edac_mc.c

Lines changed: 18 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -66,14 +66,12 @@ unsigned int edac_dimm_info_location(struct dimm_info *dimm, char *buf,
6666
char *p = buf;
6767

6868
for (i = 0; i < mci->n_layers; i++) {
69-
n = snprintf(p, len, "%s %d ",
69+
n = scnprintf(p, len, "%s %d ",
7070
edac_layer_name[mci->layers[i].type],
7171
dimm->location[i]);
7272
p += n;
7373
len -= n;
7474
count += n;
75-
if (!len)
76-
break;
7775
}
7876

7977
return count;
@@ -341,19 +339,16 @@ static int edac_mc_alloc_dimms(struct mem_ctl_info *mci)
341339
*/
342340
len = sizeof(dimm->label);
343341
p = dimm->label;
344-
n = snprintf(p, len, "mc#%u", mci->mc_idx);
342+
n = scnprintf(p, len, "mc#%u", mci->mc_idx);
345343
p += n;
346344
len -= n;
347345
for (layer = 0; layer < mci->n_layers; layer++) {
348-
n = snprintf(p, len, "%s#%u",
349-
edac_layer_name[mci->layers[layer].type],
350-
pos[layer]);
346+
n = scnprintf(p, len, "%s#%u",
347+
edac_layer_name[mci->layers[layer].type],
348+
pos[layer]);
351349
p += n;
352350
len -= n;
353351
dimm->location[layer] = pos[layer];
354-
355-
if (len <= 0)
356-
break;
357352
}
358353

359354
/* Link it to the csrows old API data */
@@ -1027,12 +1022,13 @@ void edac_mc_handle_error(const enum hw_event_mc_err_type type,
10271022
const char *other_detail)
10281023
{
10291024
struct dimm_info *dimm;
1030-
char *p;
1025+
char *p, *end;
10311026
int row = -1, chan = -1;
10321027
int pos[EDAC_MAX_LAYERS] = { top_layer, mid_layer, low_layer };
10331028
int i, n_labels = 0;
10341029
struct edac_raw_error_desc *e = &mci->error_desc;
10351030
bool any_memory = true;
1031+
const char *prefix;
10361032

10371033
edac_dbg(3, "MC%d\n", mci->mc_idx);
10381034

@@ -1087,6 +1083,8 @@ void edac_mc_handle_error(const enum hw_event_mc_err_type type,
10871083
*/
10881084
p = e->label;
10891085
*p = '\0';
1086+
end = p + sizeof(e->label);
1087+
prefix = "";
10901088

10911089
mci_for_each_dimm(mci, dimm) {
10921090
if (top_layer >= 0 && top_layer != dimm->location[0])
@@ -1114,12 +1112,8 @@ void edac_mc_handle_error(const enum hw_event_mc_err_type type,
11141112
p = e->label;
11151113
*p = '\0';
11161114
} else {
1117-
if (p != e->label) {
1118-
strcpy(p, OTHER_LABEL);
1119-
p += strlen(OTHER_LABEL);
1120-
}
1121-
strcpy(p, dimm->label);
1122-
p += strlen(p);
1115+
p += scnprintf(p, end - p, "%s%s", prefix, dimm->label);
1116+
prefix = OTHER_LABEL;
11231117
}
11241118

11251119
/*
@@ -1141,25 +1135,25 @@ void edac_mc_handle_error(const enum hw_event_mc_err_type type,
11411135
}
11421136

11431137
if (any_memory)
1144-
strcpy(e->label, "any memory");
1138+
strscpy(e->label, "any memory", sizeof(e->label));
11451139
else if (!*e->label)
1146-
strcpy(e->label, "unknown memory");
1140+
strscpy(e->label, "unknown memory", sizeof(e->label));
11471141

11481142
edac_inc_csrow(e, row, chan);
11491143

11501144
/* Fill the RAM location data */
11511145
p = e->location;
1146+
end = p + sizeof(e->location);
1147+
prefix = "";
11521148

11531149
for (i = 0; i < mci->n_layers; i++) {
11541150
if (pos[i] < 0)
11551151
continue;
11561152

1157-
p += sprintf(p, "%s:%d ",
1158-
edac_layer_name[mci->layers[i].type],
1159-
pos[i]);
1153+
p += scnprintf(p, end - p, "%s%s:%d", prefix,
1154+
edac_layer_name[mci->layers[i].type], pos[i]);
1155+
prefix = " ";
11601156
}
1161-
if (p > e->location)
1162-
*(p - 1) = '\0';
11631157

11641158
edac_raw_mc_handle_error(e);
11651159
}

0 commit comments

Comments
 (0)