makeStoreWritable: Ask forgiveness, not permission
It is surprisingly impossible to check if a mountpoint is a bind mount on Linux, and in my previous commit I forgot to check if /nix/store was even a mountpoint at all. statvfs.f_flag is not populated with MS_BIND (and even if it were, my check was wrong in the previous commit). Luckily, the semantics of mount with MS_REMOUNT | MS_BIND make both checks unnecessary: if /nix/store is not a mountpoint, then mount will fail with EINVAL, and if /nix/store is not a bind-mount, then it will not be made writable. Thus, if /nix/store is not a mountpoint, we fail immediately (since we don't know how to make it writable), and if /nix/store IS a mountpoint but not a bind-mount, we fail at first write (see below for why we can't check and fail immediately). Note that, due to what is IMO buggy behavior in Linux, calling mount with MS_REMOUNT | MS_BIND on a non-bind readonly mount makes the mountpoint appear writable in two places: In the sixth (but not the 10th!) column of mountinfo, and in the f_flags member of struct statfs. All other syscalls behave as if the mount point were still readonly (at least for Linux 3.9-rc1, but I don't think this has changed recently or is expected to soon). My preferred semantics would be for MS_REMOUNT | MS_BIND to fail on a non-bind mount, as it doesn't make sense to remount a non bind-mount as a bind mount.
This commit is contained in:
		
							parent
							
								
									2c9cf50746
								
							
						
					
					
						commit
						cc63db1dd5
					
				
					 1 changed files with 2 additions and 2 deletions
				
			
		| 
						 | 
					@ -437,12 +437,12 @@ void LocalStore::makeStoreWritable()
 | 
				
			||||||
{
 | 
					{
 | 
				
			||||||
#if HAVE_UNSHARE && HAVE_STATVFS && HAVE_SYS_MOUNT_H && defined(MS_BIND) && defined(MS_REMOUNT)
 | 
					#if HAVE_UNSHARE && HAVE_STATVFS && HAVE_SYS_MOUNT_H && defined(MS_BIND) && defined(MS_REMOUNT)
 | 
				
			||||||
    if (getuid() != 0) return;
 | 
					    if (getuid() != 0) return;
 | 
				
			||||||
    /* Check if /nix/store is a read-only bind mount. */
 | 
					    /* Check if /nix/store is on a read-only mount. */
 | 
				
			||||||
    struct statvfs stat;
 | 
					    struct statvfs stat;
 | 
				
			||||||
    if (statvfs(settings.nixStore.c_str(), &stat) !=0)
 | 
					    if (statvfs(settings.nixStore.c_str(), &stat) !=0)
 | 
				
			||||||
        throw SysError("Getting info of nix store mountpoint");
 | 
					        throw SysError("Getting info of nix store mountpoint");
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    if (stat.f_flag & (ST_RDONLY | MS_BIND)) {
 | 
					    if (stat.f_flag & ST_RDONLY) {
 | 
				
			||||||
        if (unshare(CLONE_NEWNS) == -1)
 | 
					        if (unshare(CLONE_NEWNS) == -1)
 | 
				
			||||||
            throw SysError("setting up a private mount namespace");
 | 
					            throw SysError("setting up a private mount namespace");
 | 
				
			||||||
 | 
					
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue