[PATCH 01/11] params: bound array element output to the caller's page buffer

Kees Cook <[email protected]>
Newsgroups gmane.linux.ports.ppc.embedded,gmane.linux.kernel.stable,gmane.comp.emulators.kvm.devel,gmane.linux.kernel.modules,gmane.linux.kernel.mm,gmane.comp.security.apparmor,gmane.linux.kernel.lsm,gmane.linux.uml.devel,gmane.linux.acpi.devel,gmane.linux.kernel.openipmi,gmane.comp.emulators.qemu,gmane.comp.freedesktop.xorg.drivers.intel,gmane.comp.video.dri.devel,gmane.linux.drivers.rdma,gmane.linux.drivers.video-input-infrastructure,gmane.linux.kernel.pci,gmane.linux.scsi,gmane.linux.power-management.general,gmane.linux.serial,gmane.linux.usb.general,gmane.linux.kernel,gmane.linux.kernel.cross-arch,gmane.linux.network,gmane.linux.file-systems
Message-ID <[email protected]>
From: Pengpeng Hou <[email protected]>

param_array_get() appends each element's string representation into the
shared sysfs page buffer by passing buffer + off to the element getter.

That works for getters that only write a small bounded string, but
param_get_charp() and similar helpers format against PAGE_SIZE from the
pointer they receive. Once off is non-zero, an element getter can
therefore write past the end of the original sysfs page buffer.

Collect each element into a temporary PAGE_SIZE buffer first and then
copy only the remaining space into the caller's page buffer.

Cc: [email protected]
Reviewed-by: Petr Pavlu <[email protected]>
Signed-off-by: Pengpeng Hou <[email protected]>
Signed-off-by: Kees Cook <[email protected]>
---
 kernel/params.c | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/kernel/params.c b/kernel/params.c
index 74d620bc2521..752721922a15 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -475,22 +475,36 @@ static int param_array_set(const char *val, const struct kernel_param *kp)
 static int param_array_get(char *buffer, const struct kernel_param *kp)
 {
 	int i, off, ret;
+	char *elem_buf;
 	const struct kparam_array *arr = kp->arr;
 	struct kernel_param p = *kp;
 
+	elem_buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
+	if (!elem_buf)
+		return -ENOMEM;
+
 	for (i = off = 0; i < (arr->num ? *arr->num : arr->max); i++) {
-		/* Replace \n with comma */
-		if (i)
-			buffer[off - 1] = ',';
 		p.arg = arr->elem + arr->elemsize * i;
 		check_kparam_locked(p.mod);
-		ret = arr->ops->get(buffer + off, &p);
+		ret = arr->ops->get(elem_buf, &p);
 		if (ret < 0)
-			return ret;
+			goto out;
+		ret = min(ret, (int)(PAGE_SIZE - 1 - off));
+		if (!ret)
+			break;
+		/* Replace the previous element's trailing newline with a comma. */
+		if (i)
+			buffer[off - 1] = ',';
+		memcpy(buffer + off, elem_buf, ret);
 		off += ret;
+		if (off == PAGE_SIZE - 1)
+			break;
 	}
 	buffer[off] = '\0';
-	return off;
+	ret = off;
+out:
+	kfree(elem_buf);
+	return ret;
 }
 
 static void param_array_free(void *arg)
-- 
2.34.1
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.