Re: [PATCH 08/10] parisc: fix livelock in uaccess

Helge Deller
2023-02-28T20:00:40Z

On 2/28/23 20:32, Helge Deller wrote:
> Hi Al,
>
> On 2/28/23 20:14, Al Viro wrote:
>> On Tue, Feb 28, 2023 at 07:26:48PM +0100, Helge Deller wrote:
>>
>>> I can test both parisc32 and parisc64.
>>>
>>>>     Just to confirm: your "can be killed with ^C" had been on the
>>>> mainline parisc kernel (with userfaultfd enable, of course, or it wouldn't
>>>> hang up at all), right?
>>>
>>> It was a recent mainline kernel with your patch.
>>
>> Er...  Reproducer *is* supposed to block; the bug is that on the kernel
>> without this patch it would (AFAICS) be impossible to kill - not even
>> with kill -9.  With bug fixed the behaviour should be "blocks and is
>> killable", i.e. what you've reported.
>>
>> What does it do on unpatched kernel?  *IF* the big is there, it would
>> block and be unkillable by any means.  Could you verify that?
>
> I just tried 32- and 64-bit kernels.
> With and without your patch on top of kernel 6.2.
> The result is always the same:
> The process hangs, but does not consume CPU and can be killed with ^C or kill command.
>
> strace output is:
>
> userfaultfd(0)                          = 3
> ioctl(3, UFFDIO_API, {api=0xaa, features=0 => features=UFFD_FEATURE_EVENT_FORK|UFFD_FEATURE_EVENT_REMAP|UFFD_FEATURE_EVENT_REMOVE|UFFD_FEATURE_MISSING_HUGETLBFS|UFFD_FEATURE_MISSING_SHMEM|UFFD_FEATURE_EVENT_UNMAP|UFFD_FEATURE_SIGBUS|UFFD_FEATURE_THREAD_ID|0x800, ioctls=1<<_UFFDIO_REGISTER|1<<_UFFDIO_UNREGISTER|1<<_UFFDIO_API}) = 0
> ioctl(3, UFFDIO_REGISTER, {range={start=0xf7afa000, len=0x1000}, mode=UFFDIO_REGISTER_MODE_MISSING, ioctls=1<<_UFFDIO_WAKE|1<<_UFFDIO_COPY|1<<_UFFDIO_ZEROPAGE}) = 0
> fstatfs64(0, 88, {f_type=DEVPTS_SUPER_MAGIC, f_bsize=4096, f_blocks=0, f_bfree=0, f_bavail=0, f_files=0, f_ffree=0, f_fsid={val=[0, 0]}, f_namelen=255, f_frsize=4096, f_flags=ST_VALID|ST_NOSUID|ST_NOEXEC|ST_RELATIME}) = 0

Ok, one step further...

We get fooled by glibc, which replaces this line in the testcase:
ret = fstatfs(0, (struct statfs *)mem);
and instead executes the 64-bit variant fstatfs64() inside a wrapper in glibc on another address.

I replaced that line in the testcase to use the 32-bit syscall with the right (userfault-specified) address by
ret = syscall(__NR_fstatfs, 0, mem);
and now I see:
userfaultfd(0) = 3
ioctl(3, UFFDIO_API, {api=0xaa, features=0 => features=UFFD_FEATURE_EVENT_FORK|UFFD_FEATURE_EVENT_REMAP|UFFD_FEATURE_EVENT_REMOVE|UFFD_FEATURE_MISSING_HUGETLBFS|UFFD_FEATURE_MISSING_SHMEM|UFFD_FEATURE_EVENT_UNMAP|UFFD_FEATURE_SIGBUS|UFFD_FEATURE_THREAD_ID|0x800, ioctls=1<<_UFFDIO_REGISTER|1<<_UFFDIO_UNREGISTER|1<<_UFFDIO_API}) = 0
ioctl(3, UFFDIO_REGISTER, {range={start=0xf7afa000, len=0x1000}, mode=UFFDIO_REGISTER_MODE_MISSING, ioctls=1<<_UFFDIO_WAKE|1<<_UFFDIO_COPY|1<<_UFFDIO_ZEROPAGE}) = 0
fstatfs(0,
<here it hangs now and is unkillable>

Needs further testing.

Helge