On Fri, Jun 18, 2021 at 08:29:57PM +0100, Guillaume Tucker wrote: > There's a bit more to it... The lkdtm tests make use of the > process substitution feature with the <() syntax which is > specific to Bash. The tests run by KernelCI use Debian, where > /bin/sh points to /bin/dash by default which doesn't support this > feature. So one way to fix it would be: > > (/bin/bash -c 'cat <(echo '"$test"') >'"$TRIGGER") Argh. I always forget that <() is a bash-ism. Thank you for tracking this down! > However, this might break others' workflows. > > In fact the LAVA jobs run by KernelCI do define the $SHELL > environment variable except it's defined to be /bin/sh - and that > means /bin/dash gets called and we're back to the issue explained > above. > > I've manually run a modified test job which defines > SHELL=/bin/bash and that works: > > https://lava.collabora.co.uk/scheduler/job/4055547#L2835 Yay!! > So to avoid hitting the same issue in other places, as it seems > like there is an implicit dependency on Bash, we can just change > KernelCI kselftest jobs to always export SHELL=/bin/bash. > > I suppose an even better fix would be to use standard shell > features that would work with any /bin/sh implementation, but > this is there to kill the sub-shell rather than the main script > process so I'm not entirely sure if we can easily do that > differently. Maybe we can pipe the output to cat rather than the > substitution syntax, e.g.: > > (/bin/sh -c '(echo '"$test"') | cat >'"$TRIGGER") || true Yeah, this is the right fix. There's no reason anything should depend on bash; I was just not thinking when I wrote this originally. :) > So I think the "safest" solution is to not change the kselftest > script and export SHELL=/bin/bash in the KernelCI jobs. If the > pipe approach is good enough at catching signals then it could be > done on top of this patch as it's standard and should work with > any /bin/sh implementation. What do you think? If you set SHELL=/bin/bash for now, the lkdtm tests should work as they are, and once the v2 patch lands, they'll continue to work, and SHELL=/bin/bash can be removed. Thank you so much! -Kees -- Kees Cook
