Fix zfsctl_snapshot_{,un}mount() issues #5250
Conversation
Call mount and umount via /usr/bin/env instead of /bin/sh in zfsctl_snapshot_mount() and zfsctl_snapshot_unmount(). This change fixes a shell code injection flaw. The call to /bin/sh passed the mountpoint unescaped, only surrounded by single quotes. A mountpoint containing one or more single quotes would cause the command to fail or potentially execute arbitrary shell code. This change also provides compatibility with grsecurity patches. Grsecurity only allows call_usermodehelper() to use helper binaries in certain paths. /usr/bin/* is allowed, /bin/* is not.
|
@stiell, thanks for your PR! By analyzing the history of the files in this pull request, we identified @behlendorf, @dweeezil and @nedbass to be potential reviewers. |
|
Tested on Gentoo |
| dprintf("unmount; path=%s\n", se->se_path); | ||
| error = call_usermodehelper(argv[0], argv, envp, UMH_WAIT_PROC); | ||
| strfree(argv[2]); | ||
| zfsctl_snapshot_rele(se); |
There was a problem hiding this comment.
What's the point of this patch when your other patch negate the possibility of use-after-free?
There was a problem hiding this comment.
Oh sorry, the use-after-free is in dprintf not strfree. Then you should just move the zfsctl_snapshot_rele right after dprintf.
There was a problem hiding this comment.
The second patch also uses se in argv, so then zfsctl_snapshot_rele would have to be moved to after call_usermodehelper again in that patch.
There was a problem hiding this comment.
@tuxoko can you approve the patch if you're happy with this.
Fix use after free in
zfsctl_snapshot_unmount(). Use/usr/bin/envinstead of/bin/shto fix a shell code injection flaw and allow use with grsecurity.Not yet tested on a kernel with grsecurity.
Fixes #4377. Supersedes #4568 and #4573.