dracut cannot handle whitespace in a pool name

[dajhorn]

A literal space character is permitted in a zpool name. For example:

# zpool create 'foo bar' /dev/sdb
# zfs list
NAME                  USED  AVAIL  REFER  MOUNTPOINT
foo bar                75k  9.78G    30K  /foo bar

Unquoted variables near dracut/90zfs/mount-zfs.sh.in:64 like this:

mountpoint=`zfs get -H -o value mountpoint $zfsbootfs`

Get a command substitution like this:

zfs get -H -o value mountpoint foo bar/ROOT/linux
cannot open 'foo': dataset does not exist
cannot open 'bar/ROOT/linux': dataset does not exist

[behlendorf]

Now this is a bug, the pool name should not be allowed to contain white space. How this is slipping through isn't immediately obvious because there is code to detect this sort of thing. See zpool_name_valid()->pool_namecheck()->valid_char() in the source. Allowed characters are a-z, A-Z, 0-9, -, _, ., and :. Passing a name which include a space should be caught.

[Rudd-O]

Pool names cannot contain spaces?

Still, it'd be a good idea to quote variables in the dracut shell scripts. Better safe than sorry. Dajhorn, can you submit a fix in the form of a pull request to either my tree or Brian's?

[dajhorn]

I confirmed that a pool with whitespace in the name can be created on both Fedora 16, Ubuntu 11.10, and OpenIndiana 151a.

If this is a bug, then it is in OpenSolaris/Illumos too.

[behlendorf]

Interesting, well it certainly still seems like a bug to me even if it is allowed by the other platforms. How do you guys want to handle this, fix the code to prevent the creation of pools with white space in their names. Or update all of the scripts/utilities to quote everything.

Preventing this entirely and pushing the fix back to Illumos seems like the best fix to me. There must be a ton of 3rd party scripts which also make this assumption.

[Rudd-O]

I think the solution with the most closure is to quote all variables in
our scripts. That would allow us to handle crazy stuff we can't expect
robustly.

On Mon, 2011-11-14 at 12:51 -0800, Brian Behlendorf wrote:

Interesting, well it certainly still seems like a bug to me even if it is allowed by the other platforms. How do you guys want to handle this, fix the code to prevent the creation of pools with white space in their names. Or update all of the scripts/utilities to quote everything.

Preventing this entirely and pushing the fix back to Illumos seems like the best fix to me. There must be a ton of 3rd party scripts which also make this assumption.

---

Reply to this email directly or view it on GitHub:
#439 (comment)

[schakrava]

I agree with Rudd-O. I don't see a reason to disallow same character set for zpool names and zfs filesystem names. Not allowing space character might actually cause problems with some potential use cases. Also looking at the code, i do see where its allowing the space character: zfs_namecheck.c:valid_char() line #55. Its the last character comparison.

[dajhorn]

In the same source file, the comment for snapshot_namecheck() gives a regex example that includes the space character, so it is probably intentional.

[behlendorf]

OK, you've convinced me. It's intentional. If someone can provide a patch which adds the needed quoting to the relevant scripts I'm happy to pull it in.

[behlendorf]

This looks stale, and it should have been fixed by 660cbad. Closing issue.