Comment 28 for bug 1476662

Tyler Hicks (tyhicks) wrote :

Hi Serge - I've reviewed the patch against git HEAD and have a couple questions.

1) In mount_entry(), the call to mount(2) is changed to use target as both the source and the destination. I can't see why this would be a valid change.

2) In is_file_mount(), the usage of lstat(2) seems odd for a couple reasons:
   - If path is a symlink to a directory, the mount will be incorrectly treated as a file mount in safe_mount(). I think stat(2) should be used.
   - Error checking of lstat(2) is ignored. I think this should be treated as fatal for most (all?) cases.

3) In open_without_symlink(), why is curlen decremented after assigning it the return value of strlen(prefix_skip)? Is it because prefix_skip is somehow guaranteed to have a trailing '/' character?

4) open_without_symlink() is documented to return -1 upon error but it returns -EINVAL when target doesn't start with prefix_skip. (minor)

5) Is there a need to protect against ".." components in open_without_symlink()? I'm thinking of a target of "/foo/../" and a prefix_skip of "/foo".