>Number:         34499
>Category:       bin
>Synopsis:       ls allocates more memory than needed
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    bin-bug-people
>State:          open
>Class:          change-request
>Submitter-Id:   net
>Arrival-Date:   Sat Sep 09 07:10:00 +0000 2006
>Originator:     mac@S.culver.net
>Release:        NetBSD 3.0.1
>Organization:
	
>Environment:
	
	
System: NetBSD S.Culver.Net 3.0.1 NetBSD 3.0.1 (S) #0: Sun Jul 30 02:26:46 PDT 2006 mac@S.Culver.Net:/usr/obj/sys/arch/i386/compile/S i386
Architecture: i386
Machine: i386
>Description:
	too large by one byte malloc()
>How-To-Repeat:
	Look at ls.h and ls.c
>Fix:

560c560
<                                   ulen + glen + flen + 3)) == NULL)
---
>                                   ulen + glen + flen + 2)) == NULL)


------------
Note that the NAMES structure, defined in ls.h is:

typedef struct {
	char *user;
	char *group;
	char *flags;
	char data[1];
} NAMES;

and when used in ls.c:


				if ((np = malloc(sizeof(NAMES) +
				    ulen + glen + flen + 3)) == NULL)
					err(EXIT_FAILURE, NULL);

				np->user = &np->data[0];
				(void)strcpy(np->user, user);
				np->group = &np->data[ulen + 1];
				(void)strcpy(np->group, group);

				if (f_flags) {
					np->flags = &np->data[ulen + glen + 2];
				  	(void)strcpy(np->flags, flags);
				}
				cur->fts_pointer = np;


Notice that the original malloc allocates an additional byte  (ulen + glen + flen + 3) because it doesn't take into account the fact that sizeof(NAMES) already includes 1 extra byte for the char data[1] element.

Therefore, for 3 null-terminated strings, only sizeof(NAMES) + ulen + glen + flen + 2 bytes should be allocated.

This was orignally pointed out by Diomidis Spinellis in "Code Reading", which is on p 90 of the 2nd edition.

>Unformatted: