On Wed, Feb 21, 2024 at 03:09:51PM +0100, David Sterba wrote: > Hi, > > reading [1] and how late it was found that effectively a "rm -rf /" can > happen makes me worried about what I can expect from fstests after git > pull. Many people contribute and the number for custom _cleanup() > functions with unquoted 'rm' commands is just asking for more problems. > > [1] https://lore.kernel.org/all/20240205060016.7fgiyafbnrvf5chj@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/ > > Unquoted arguments in shell scripts is IMO a big anti-pattern, > unfortunately present everywhere in xfstests since the beginning. > Rewriting all scripts would be quite a lot of work, could you at least > provide safe versions of the cleanup helpers? Hi David, Thanks for taking care about it :) > > For example: > > _rm_tmp() { > rm -rf -- $tmp It's "$tmp.*" May I ask what problem does the "--" hope to avoid? If the "$tmp" is empty, "rm -rf" and "rm -rf --"" looks like both doing nothing. So what kind of situation does the "--" hope to fix? The root problem in above [1] is about "${FOO}*". If someone does "rm -rf ${FOO}*" in its custom _cleanup_xxxxx function, then it's dangerous if "$FOO" is empty. I thought some ways to avoid that: 1) Try to avoid doing rm -rf ${FOO}*, if not necessary. 2) Must checks [ -n "$FOO" ] before doing any rm -rf ${FOO}* 3) Someone's custom _cleanup_xxxxx better to be called before default _cleanup does "cd /". 4) Think about bringing in someone "Static program analysis" tool about bash script, but I don't know if there're someone good, feel free to give me suggestions. > } > > and used as > > _cleanup() { > _rm_tmp > } > > or at least mandate the "--" separator and quoting arguments in new code > and gradually fix the existing code. > > I can send patches at least for btrfs and generic as this affects me but > first I'd like to know that this will become standard coding style > requirement in fstests. If you'd like to send a patch to cleanup the cleanup things, that's welcome. (just don't bring in regression issue, please:) >
