BUG: cause segmentfault when stat() passed invalid parameter

[Xishi Qiu]

On 2014/8/30 13:35, Rich Felker wrote:

> On Sat, Aug 30, 2014 at 10:56:26AM +0800, Xishi Qiu wrote:
>> Here is my test code.
>> #include <sys/types.h>
>> #include <stdio.h>
>> #include <unistd.h>
>> #include <sys/stat.h>
>>
>> int main(int ac, char **av)
>> {
>>     int test;
>>     char *path = "/tmp/stat01";  // I have created stat01 before
>>     test = stat(path, -1);
>>     printf("stat=%d\n", test);
>>     return 0;
>> }
>>
>> Then I use strace to trace it.
>> ....
>> ioctl(0, SNDCTL_TMR_TIMEBASE or SNDRV_TIMER_IOCTL_NEXT_DEVICE or TCGETS, {B115200 opost isig icanon echo ...}) = 0
>> ioctl(1, SNDCTL_TMR_TIMEBASE or SNDRV_TIMER_IOCTL_NEXT_DEVICE or TCGETS, {B115200 opost isig icanon echo ...}) = 0
>> stat64("/tmp/stat01", {st_mode=S_IFREG|0644, st_size=0, ...}) = 0
>> --- SIGSEGV {si_signo=SIGSEGV, si_code=SEGV_MAPERR, si_addr=0xffffffff} ---  // This is the segmentfault
>> +++ killed by SIGSEGV +++
>> sh-4.2#
>>
>> Here is uclibc code.
>> int stat(const char *file_name, struct stat *buf)
>> {
>> 	int result;
>> # ifdef __NR_stat64
>> 	/* normal stat call has limited values for various stat elements
>> 	 * e.g. uid device major/minor etc.
>> 	 * so we use 64 variant if available
>> 	 * in order to get newer versions of stat elements
>> 	 */
>> 	struct kernel_stat64 kbuf;
>> 	result = INLINE_SYSCALL(stat64, 2, file_name, &kbuf);  // return success
>> 	if (result == 0) {
>> 		__xstat32_conv(&kbuf, buf);  // memset address(-1) cause the segmentfault
>> 	}
>> # else
>> 	struct kernel_stat kbuf;
>>
>> 	result = INLINE_SYSCALL(stat, 2, file_name, &kbuf);
>> 	if (result == 0) {
>> 		__xstat_conv(&kbuf, buf);
>> 	}
>> # endif /* __NR_stat64 */
>> 	return result;
>> }
>> #endif /* __NR_fstat64 */
>> libc_hidden_def(stat)
>>
>> Is this a bug or we should not pass a invalid parameter to stat()?
> 
> Would you expect it to work if you passed (struct stat *)rand() to
> stat? No, because it would potentially clobber a random part of your
> program's memory. So why should passing -1 be safe? This is not a bug
> in uclibc, just in the caller that's passing an invalid pointer, which
> is (always) undefined behavior.
> 
> Rich

Hi Rich,

Thanks for your reply.

But in x86(use glibc), I tested the program too, it just return an error number,
not segment fault. I think glibc direct use the user's buf, and syscall return faild.
The code may be like this:
  stat()
    ...
    return INLINE_SYSCALL(stat64, 2, file_name, buf); 
    ...

Here is the log from strace in x86(glibc):
stat("/tmp/stat01", 0xffffffffffffffff) = -1 EFAULT (Bad address)  // -> syscall return faild
fstat(1, {st_mode=S_IFCHR|0620, st_rdev=makedev(136, 20), ...}) = 0
mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7fccf677f000
write(1, "stat=-1\n", 8stat=-1
)                = 8
exit_group(0)                           = ?

Well, do you mean this is not a bug, and need not to change the uclibc?

Thanks,
Xishi Qiu