Skip to content

Commit cc7a0bb

Browse files
tyreldmpe
authored andcommitted
PCI: rpadlpar: Fix potential drc_name corruption in store functions
Both add_slot_store() and remove_slot_store() try to fix up the drc_name copied from the store buffer by placing a NUL terminator at nbyte + 1 or in place of a '\n' if present. However, the static buffer that we copy the drc_name data into is not zeroed and can contain anything past the n-th byte. This is problematic if a '\n' byte appears in that buffer after nbytes and the string copied into the store buffer was not NUL terminated to start with as the strchr() search for a '\n' byte will mark this incorrectly as the end of the drc_name string resulting in a drc_name string that contains garbage data after the n-th byte. Additionally it will cause us to overwrite that '\n' byte on the stack with NUL, potentially corrupting data on the stack. The following debugging shows an example of the drmgr utility writing "PHB 4543" to the add_slot sysfs attribute, but add_slot_store() logging a corrupted string value. drmgr: drmgr: -c phb -a -s PHB 4543 -d 1 add_slot_store: drc_name = PHB 4543°|<82>!, rc = -19 Fix this by using strscpy() instead of memcpy() to ensure the string is NUL terminated when copied into the static drc_name buffer. Further, since the string is now NUL terminated the code only needs to change '\n' to '\0' when present. Cc: [email protected] Signed-off-by: Tyrel Datwyler <[email protected]> [mpe: Reformat change log and add mention of possible stack corruption] Signed-off-by: Michael Ellerman <[email protected]> Link: https://lore.kernel.org/r/[email protected]
1 parent eed5fae commit cc7a0bb

File tree

1 file changed

+6
-8
lines changed

1 file changed

+6
-8
lines changed

drivers/pci/hotplug/rpadlpar_sysfs.c

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -34,12 +34,11 @@ static ssize_t add_slot_store(struct kobject *kobj, struct kobj_attribute *attr,
3434
if (nbytes >= MAX_DRC_NAME_LEN)
3535
return 0;
3636

37-
memcpy(drc_name, buf, nbytes);
37+
strscpy(drc_name, buf, nbytes + 1);
3838

3939
end = strchr(drc_name, '\n');
40-
if (!end)
41-
end = &drc_name[nbytes];
42-
*end = '\0';
40+
if (end)
41+
*end = '\0';
4342

4443
rc = dlpar_add_slot(drc_name);
4544
if (rc)
@@ -65,12 +64,11 @@ static ssize_t remove_slot_store(struct kobject *kobj,
6564
if (nbytes >= MAX_DRC_NAME_LEN)
6665
return 0;
6766

68-
memcpy(drc_name, buf, nbytes);
67+
strscpy(drc_name, buf, nbytes + 1);
6968

7069
end = strchr(drc_name, '\n');
71-
if (!end)
72-
end = &drc_name[nbytes];
73-
*end = '\0';
70+
if (end)
71+
*end = '\0';
7472

7573
rc = dlpar_remove_slot(drc_name);
7674
if (rc)

0 commit comments

Comments
 (0)