canbusload: pselect() is called with uninitialzied sigmask

[marckleinebudde]

This causes random hangs, as the alarm signal might be blocked.

https://github.com/linux-can/can-utils/blob/c5b64802a18451c2dc4b31063ac4529766babae8/canbusload.c#L397

@hartkopp Do you know what the original rationale behind this was?

[hartkopp]

@marckleinebudde I was using pselect() in that time to handle the signals properly within one syscall.

I've never seen any issues but I only used CRTL-C (Sigterm?) so far .

[marckleinebudde]

With current master, canbusload sometimes hangs. sigmask is uninitialized and may randomly block ALRM, as seen in this example:

$ strace -e pselect6 ./canbusload -t -c -b -r -e vcan0@500000
pselect6(4, [3], NULL, NULL, NULL, {sigmask=[HUP QUIT ILL TRAP ABRT KILL SEGV USR2 ALRM TERM XCPU PROF WINCH IO PWR RT_1 RT_2 RT_3 RT_4 RT_5 RT_6 RT_7 RT_8 RT_9 RT_10 RT_11 RT_12 RT_13 RT_14 RT_15], sigsetsize=8}

The version that comes with current Debian testing works, as sigmask is NULL. Don't know if this is by accident or if Debian uses some compiler switches to initialize on stack variables by default to 0x0.

$ strace -e pselect6 ./canbusload -t -c -b -r -e vcan0@500000
pselect6(4, [3], NULL, NULL, NULL, {sigmask=NULL, sigsetsize=8}) = ? ERESTARTNOHAND (To be restarted if no handler)
--- SIGALRM {si_signo=SIGALRM, si_code=SI_KERNEL} ---

[hartkopp]

Hm, I thought sigterm and sigalarm are needed as I use a 1 second alarm to redraw the values. Interesting that it still works with a sigmask=NULL

[marckleinebudde]

The signals in sigmask are actually blocked. This is why it doesn't work, if sigmask contains a random value which blocks ALRM (see above example).

[hartkopp]

So you would suggest to set the sigmask either to zero or just the bits for sigterm and sigalarm ?

[marckleinebudde]

If you set the bit for ALRM, it does not work, as the signal would be blocked. With NULL no signal is blocked, and redraw works.

[hartkopp]

Sorry, I wanted to suggest to set "just the bits for sigterm and sigalarm" to zero, and have the others blocked. But when it just works with all bits set to zero, you might also go for this solution.