linux-kernel - Re: [PATCH] kernel: module: strncpy issue, using strlcpy instead of strncpy

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5163E123.5080205@asianux.com>
Date:	Tue, 09 Apr 2013 17:36:35 +0800
From:	Chen Gang <gang.chen@...anux.com>
To:	Rusty Russell <rusty@...tcorp.com.au>
CC:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] kernel: module: strncpy issue, using strlcpy instead
 of strncpy

On 2013年04月09日 09:52, Chen Gang wrote:
>>>>
>>>>   it looks like a bug.  for me, I prefer to give length check for it.
>>>>
>>>>   but I am sorry, now, I can not be sure whether it is really a bug.
>> It really is.  We don't export any symbols > 128 characters, but if we
>> did then kallsyms_expand_symbol() would overflow the buffer handed to
>> it.
>>
>> Your suggestion about an explicit length for kallsyms_expand_symbol() is
>> the correct one.
> 
> 

  oh, sorry, after read the related source code, I think:

    for kernel/kallsyms.c, it is no issue
      (although I still also prefer to give a length checking).

    the reason is:
      scripts/kallsyms.c does not check the 128 limitation.
      if compiler limits it with 128 automatically, we will have no issue.
      else the scripts/kallsyms will cause issue.

    so whether what happens, kernel/kallsyms.c will not cause issue.

    :-)


  the related patch for scripts/kallsyms.c is below, please check, thanks.
    (now, it is just for a reference, after have a test, I will send).


-----------------------patch begin--------------------------------------

diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
index 487ac6f..9ec6d1f 100644
--- a/scripts/kallsyms.c
+++ b/scripts/kallsyms.c
@@ -145,13 +145,15 @@ static int read_symbol(FILE *in, struct sym_entry *s)
 	/* include the type field in the symbol name, so that it gets
 	 * compressed together */
 	s->len = strlen(str) + 1;
+	if (s->len > KSYM_NAME_LEN)
+		s->len = KSYM_NAME_LEN;
 	s->sym = malloc(s->len + 1);
 	if (!s->sym) {
 		fprintf(stderr, "kallsyms failure: "
 			"unable to allocate required amount of memory\n");
 		exit(EXIT_FAILURE);
 	}
-	strcpy((char *)s->sym + 1, str);
+	strlcpy((char *)s->sym + 1, str, KSYM_NAME_LEN);
 	s->sym[0] = stype;
 
 	return 0;
@@ -290,7 +292,7 @@ static void write_src(void)
 	unsigned int i, k, off;
 	unsigned int best_idx[256];
 	unsigned int *markers;
-	char buf[KSYM_NAME_LEN];
+	char buf[KSYM_NAME_LEN + 1];
 
 	printf("#include <asm/types.h>\n");
 	printf("#if BITS_PER_LONG == 64\n");
-- 
1.7.7.6


-----------------------patch end----------------------------------------



>   thank you to give me additional chance to send a new patch,
>   I will send another patch for it.
> 
>   since you find it, firstly, and also give a confirmation.
>   can I add you as "Signed-of-by", too ?
> 
>   and excuse me:
>     I think I should spend time resources to have a test for new patch.
>     so I will finish it within this weekend (2013-4-14), is it OK ?
> 
> 
>   thanks.
> 
> 


-- 
Chen Gang

Asianux Corporation
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/