arun11299 / cpp-subprocess

Subprocessing with modern C++
Other
449 stars 90 forks source link

crash #104

Open enenH opened 3 weeks ago

enenH commented 3 weeks ago

On Android

int main(int argc, char** argv) { auto obuf = check_output({"ls", "-l"}); printf( "obuf:%s\n", obuf.buf.data()); return 0; }

fdsan: attempted to close file descriptor 5, expected to be unowned, actually owned by FILE* 0x75c06f90b0

enenH commented 3 weeks ago

06-29 13:58:55.841 12102 12102 F DEBUG : pid: 12097, tid: 12097, name: entry >>> /data/local/tmp/entry <<< 06-29 13:58:55.841 12102 12102 F DEBUG : uid: 0 06-29 13:58:55.841 12102 12102 F DEBUG : tagged_addr_ctrl: 0000000000000001 (PR_TAGGED_ADDR_ENABLE) 06-29 13:58:55.841 12102 12102 F DEBUG : pac_enabled_keys: 000000000000000f (PR_PAC_APIAKEY, PR_PAC_APIBKEY, PR_PAC_APDAKEY, PR_PAC_APDBKEY) 06-29 13:58:55.841 12102 12102 F DEBUG : signal 6 (SIGABRT), code -1 (SI_QUEUE), fault addr -------- 06-29 13:58:55.841 12102 12102 F DEBUG : Abort message: 'fdsan: attempted to close file descriptor 5, expected to be unowned, actually owned by FILE 0x7bc28f90b0' 06-29 13:58:55.841 12102 12102 F DEBUG : x0 0000000000000000 x1 0000000000002f41 x2 0000000000000006 x3 0000007fea15f340 06-29 13:58:55.841 12102 12102 F DEBUG : x4 0080808080808080 x5 0080808080808080 x6 0080808080808080 x7 8080808080808080 06-29 13:58:55.841 12102 12102 F DEBUG : x8 00000000000000f0 x9 56b4ac894a1e475c x10 0000000000000002 x11 7f7f7f7f7f7f7f7f 06-29 13:58:55.841 12102 12102 F DEBUG : x12 0101010101010101 x13 000000007fffffff x14 0000000000061e00 x15 0000000000000010 06-29 13:58:55.841 12102 12102 F DEBUG : x16 0000007c4ddd1a68 x17 0000007c4ddaa710 x18 0000007c6dfc8000 x19 0000000000002f41 06-29 13:58:55.841 12102 12102 F DEBUG : x20 0000000000002f41 x21 0000007c6d006010 x22 0000000000000003 x23 0000007fea15f138 06-29 13:58:55.841 12102 12102 F DEBUG : x24 0000007fea15f430 x25 0000007fea15f0b0 x26 0000007fea15f070 x27 ffffff80ffffffc8 06-29 13:58:55.841 12102 12102 F DEBUG : x28 0000000000000000 x29 0000007fea15f3d0 06-29 13:58:55.841 12102 12102 F DEBUG : lr 0000007c4dd56394 sp 0000007fea15eff0 pc 0000007c4dd563b8 pst 0000000000001000 06-29 13:58:55.841 12102 12102 F DEBUG : backtrace: 06-29 13:58:55.841 12102 12102 F DEBUG : #00 pc 00000000000913b8 /apex/com.android.runtime/lib64/bionic/libc.so (fdsan_error(char const, ...)+552) (BuildId: 95768e541c682c20709fc11df6431f16) 06-29 13:58:55.841 12102 12102 F DEBUG : #01 pc 00000000000910c4 /apex/com.android.runtime/lib64/bionic/libc.so (android_fdsan_close_with_tag+724) (BuildId: 95768e541c682c20709fc11df6431f16) 06-29 13:58:55.841 12102 12102 F DEBUG : #02 pc 0000000000091820 /apex/com.android.runtime/lib64/bionic/libc.so (close+16) (BuildId: 95768e541c682c20709fc11df6431f16) 06-29 13:58:55.841 12102 12102 F DEBUG : #03 pc 000000000020c64c /data/local/tmp/entry (subprocess::Popen::execute_process()+1220) (BuildId: 908c3bd30562abb0d7f0e94572fc90a5488867df) 06-29 13:58:55.841 12102 12102 F DEBUG : #04 pc 000000000021d93c /data/local/tmp/entry (subprocess::Popen::Popen(std::initializer_list<char const>, subprocess::output&&)+400) (BuildId: 908c3bd30562abb0d7f0e94572fc90a5488867df) 06-29 13:58:55.841 12102 12102 F DEBUG : #05 pc 000000000021d620 /data/local/tmp/entry (subprocess::Buffer subprocess::detail::check_output_impl<std::initializer_list<char const> >(std::initializer_list<char const>&)+108) (BuildId: 908c3bd30562abb0d7f0e94572fc90a5488867df) 06-29 13:58:55.841 12102 12102 F DEBUG : #06 pc 00000000001e6f28 /data/local/tmp/entry (subprocess::Buffer subprocess::check_output<>(std::initializer_list<char const>)+44) (BuildId: 908c3bd30562abb0d7f0e94572fc90a5488867df) 06-29 13:58:55.841 12102 12102 F DEBUG : #07 pc 00000000001e6e58 /data/local/tmp/entry (main+96) (BuildId: 908c3bd30562abb0d7f0e94572fc90a5488867df) 06-29 13:58:55.841 12102 12102 F DEBUG : #08 pc 0000000000085240 /apex/com.android.runtime/lib64/bionic/libc.so (__libc_init+96) (BuildId: 95768e541c682c20709fc11df6431f16)

arun11299 commented 2 weeks ago

@enenH Thank you for reporting.

My guess is that we need to close the FILE pointers (input, output, error_) as well before close the file descriptors directly in close_parent_fds function.

Are you comfortable to try this and check again ? My valgrind with fd tracking did not give any issues on ubuntu.

Thank you.

enenH commented 2 weeks ago

void close_parent_fds() { if(input()) fclose(input()); if(output()) fclose(output()); if(error()) fclose(error()); if (write_tochild != -1) close(write_tochild); if (read_fromchild != -1) close(read_fromchild); if (errread != -1) close(errread); }

Same problem

arun11299 commented 2 weeks ago

Thanks for trying it. Is the fdsan error exactly the same or is there any difference ?

enenH commented 2 weeks ago

https://github.com/arun11299/cpp-subprocess/blob/4025693decacaceb9420efedbf4967a04cb028e7/subprocess.hpp#L1637 This code is causing the crash. The following code works fine. auto f=fdopen(err_rd_pipe, "r"); int read_bytes = util::read_atmost_n(f, err_buf, SP_MAX_ERR_BUF_SIZ); if (f) fclose( f); close(err_rd_pipe); If fdopen() is successful, you must use fclose() to close the stream and file

arun11299 commented 2 weeks ago

Great! If you can open a PR I would be happy to merge it.