Skip to contentSkip to conversation

check d_path() error in print-fatal-signals

PATCH In review Akinobu Mita 2 participants 2 messages May 20, 2007

lore
Share & tools
lore

Akinobu Mita

author
#0 lore raw

PATCH 1/1 check d_path() error in print-fatal-signals

d_path() returns -ENAMETOOLONG if buffer length is not enough. But there is no error handling for it in print_vma() which calls d_path() with not enough buffer (We can easily make segfault program which has longer path than 128bytes).

This patch allocates enough buffer for d_path() dynamically. audit_log_d_path() is doing sililar thing. So I just stole from it.

Signed-off-by Akinobu Mita <[email protected]>
---
 kernel/signal.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)
kernel/signal.c +15 -7
Index: 2.6-mm/kernel/signal.c
===================================================================
@@ -740,15 +740,23 @@ static int print_vma(struct vm_area_stru
 	 * special [heap] marker for the heap:
 	 */
 	if (file) {
-#define SIZE 128
-		char tmp[SIZE], *str;
-
-		str = d_path(file->f_dentry, file->f_vfsmnt, tmp, SIZE);
-		while (str[0] && (str[0] == ' '))
-			str++;
+		char *p, *path;
 
+		/* We will allow 11 spaces for ' (deleted)' to be appended */
+		path = kmalloc(PATH_MAX + 11, GFP_KERNEL);
+		if (!path)
+			p = "<no memory>";
+		else {
+			p = d_path(file->f_dentry, file->f_vfsmnt, path,
+				   PATH_MAX + 11);
+			if (IS_ERR(p))
+				p = "<too long>";
+			else
+				p = strstrip(p);
+		}
 		pad_len_spaces(len);
-		printk("%s", str);
+		printk("%s", p);
+		kfree(path);
 	} else {
 		const char *name = arch_vma_name(vma);
 		if (!name) {
#1 lore raw
Quoted 39 lines · On Sun, 20 May 2007 14:56:39 +0900 Akinobu Mita <[email protected]> wrote:

d_path() returns -ENAMETOOLONG if buffer length is not enough. But there is no error handling for it in print_vma() which calls d_path() with not enough buffer (We can easily make segfault program which has longer path than 128bytes).

This patch allocates enough buffer for d_path() dynamically. audit_log_d_path() is doing sililar thing. So I just stole from it.

Signed-off-by Akinobu Mita <[email protected]>
---
 kernel/signal.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

Index: 2.6-mm/kernel/signal.c
===================================================================

kernel/signal.c +10 -6
@@ -740,15 +740,23 @@ static int print_vma(struct vm_area_stru
 	 * special [heap] marker for the heap:
 	 */
 	if (file) {
-#define SIZE 128
-		char tmp[SIZE], *str;
-
-		str = d_path(file->f_dentry, file->f_vfsmnt, tmp, SIZE);
-		while (str[0] && (str[0] == ' '))
-			str++;
+		char *p, *path;
 
+		/* We will allow 11 spaces for ' (deleted)' to be appended */
+		path = kmalloc(PATH_MAX + 11, GFP_KERNEL);
+		if (!path)
+			p = "<no memory>";
+		else {
+			p = d_path(file->f_dentry, file->f_vfsmnt, path,
+				   PATH_MAX + 11);
+			if (IS_ERR(p))
+				p = "<too long>";

kernel/signal.c static int print_vma(struct vm_area_stru · line 762

Quoted 10 lines
kernel/signal.c +5 -1
+			else
+				p = strstrip(p);
+		}
 		pad_len_spaces(len);
-		printk("%s", str);
+		printk("%s", p);
+		kfree(path);
 	} else {
 		const char *name = arch_vma_name(vma);
 		if (!name) {

Thanks.

This bug exists only in -mm kernels, and is introduced by the -mm-only debugging patch vdso-improve-print_fatal_signals-support-by-adding-memory-maps.patch.

I think I'll just drop that patch - I don't think it is proving useful to anyone.