Re: fsck segfaults on rpi3 running 13-stable (and on 14-CURRENT analyzing the same file system that resulted from the 13-STABLE crash)

On Feb 13, 2023, at 15:25, John-Mark Gurney <jmg@funkthat.com> wrote:

> Mark Millard wrote this message on Sun, Feb 12, 2023 at 13:25 -0800:
>> [With a backtrace for the fsck_ffs SIGSEGV crash and some
>> listing of code involved, I'm now including mckusick@FreeBSD.org <mailto:mckusick@FreeBSD.org>
>> in the To: . Kirk M. likely would like you to preserve the
>> problematical UFS file system that produces the fsck_ffs
>> crashes, at least for now. For Kirk M.: The below is from/for
>> the fsck_ffs attempted from 14-CURRENT.]
>> 
>> On Feb 12, 2023, at 11:53, bob prohaska <fbsd@www.zefox.net> wrote:
>> 
>>> On Sun, Feb 12, 2023 at 11:31:59AM -0800, Mark Millard wrote:
>>>> 
>>>> I'll note that another option is to run fsck_ffs from
>>>> lldb in the first place. 
>>> 
>>> That seems more productive, yielding:
> 
> [...]
> 
>> So the code around /usr/main-src/sbin/fsck_ffs/inode.c:1314 looks
>> like: (leading white space might not be preserved)
>> 
>> void
>> prtinode(struct inode *ip)
>> {
>>        char *p;
>>        union dinode *dp;
>>        struct passwd *pw;
>>        time_t t;
>>          dp = ip->i_dp;
>>        printf(" I=%lu ", (u_long)ip->i_number);
>>        if (ip->i_number < UFS_ROOTINO || ip->i_number > maxino)
>>                return;
>>        printf(" OWNER=");
>>        if ((pw = getpwuid((int)DIP(dp, di_uid))) != NULL)
>>                printf("%s ", pw->pw_name);
>>        else
>>                printf("%u ", (unsigned)DIP(dp, di_uid));
>>        printf("MODE=%o\n", DIP(dp, di_mode));
>>        if (preen)
>>                printf("%s: ", cdevname);
>>        printf("SIZE=%ju ", (uintmax_t)DIP(dp, di_size));
>>        t = DIP(dp, di_mtime);
>>        p = ctime(&t);
>>        printf("MTIME=%12.12s %4.4s ", &p[4], &p[20]);
>> }
> 
> [...]
> 
>> So far, I've not identified how the NULL pointer showed up
>> that ended up being dereferenced. It does not look likely
>> that I will identify such.
> 
> Ok, decided to run AFL on fsck, and this one was the first crash it
> discovered.  The problem is that ctime can return NULL, and the return
> value isn't checked, because it then immediately does &p[4] which
> results is printf and friends being passed 0x4.
> 
> Simple test program that demonstrates this problem:
> #include <time.h>
> #include <stdio.h>
> 
> int
> main()
> {
>        const char *p;
>        time_t t;
> 
>        t = -5098919203113507862;
> 
>        p = ctime(&t);
> 
>        printf("MTIME=%12.12s %4.4s ", &p[4], &p[20]);
> 
>        return 0;
> }
> 
> I'm not sure what the correct fix is for when times are wildly out of
> valid range.
> 

Thanks.

Looks like C23 is making ctime and asctime deprecated,
recommending strftime use.

As for existing behavior vs. standards:

C11, for example, just indicates that
ctime(t) is equivalent to asctime(localtime(t)) .

C11 also indicates that asctime goes to a
"the behavior of the asctime function is undefined"
status. NULL returns are only one of the possibilities
as far as he language is concerned.

POSIX indicates the more specific NULL return value
handling for asctime (and, so, ctime too).

===
Mark Millard
marklmi at yahoo.com