ltp icon indicating copy to clipboard operation
ltp copied to clipboard

`recvmsg01.c` doesn't have error checking in many places

Open dimakuv opened this issue 2 years ago • 6 comments

I detected this deficiency while working on https://github.com/gramineproject/gramine/pull/1210 on the Gramine project. Gramine LibOS currently doesn't support ancillary data (cmsg).

recvmsg01.c test uses ancillary data in the setup process, in particular this:

  • https://github.com/linux-test-project/ltp/blob/7602a23480d6ac6a3cff8e63342a842ef94a4b33/testcases/kernel/syscalls/recvmsg/recvmsg01.c#L508-L514
  • https://github.com/linux-test-project/ltp/blob/7602a23480d6ac6a3cff8e63342a842ef94a4b33/testcases/kernel/syscalls/recvmsg/recvmsg01.c#L522-L526

The problem is on line 526 -- sendmsg() sends a message with the SCM_RIGHTS file descriptor. There is no error checking. Thus, the sender doesn't notice if this syscall fails (like it happens in Gramine). After that, the reader process is hanged on the blocking recvmsg() here. And the whole test gets broken because of this one case (this setup happens for subtest 8).

Could you add error handling? At least for this sendmsg()? But in general, there are several other places in this test that call non-trivial syscalls and do not check their return values.

dimakuv avatar Mar 06 '23 18:03 dimakuv

Unfortunately we still have around 300 old testcases that have to be cleaned up and rewritten to use the new test library, we are down from more than 1000 when we started about five years ago. We will rewrite this test eventually as well, it may take a year to get there though.

metan-ucw avatar Mar 07 '23 10:03 metan-ucw

Sure, I understand, @metan-ucw. I just decided to create this ticket since I debugged this problem yesterday and wanted to describe this finding somewhere. Feel free to close it.

dimakuv avatar Mar 07 '23 10:03 dimakuv

@dimakuv Let's keep it open, it increases the chance that someone will pick the task. Also as usual patches are welcome :-)

metan-ucw avatar Mar 07 '23 10:03 metan-ucw

https://patchwork.ozlabs.org/project/ltp/patch/[email protected]/

coolgw avatar Mar 29 '23 13:03 coolgw

The refactoring has been merged as 7d365d72ba. I'll check whether this also covers the error handling.

Martchus avatar Sep 28 '23 10:09 Martchus

It looks like the refactoring included a fix (using SAFE_SENDMSG() instead of sendmsg()). So this issue can probably be closed.

Martchus avatar Sep 28 '23 11:09 Martchus