hugetlbfs :shmget with SHM_HUGETLB only works as root

lock_limit = current->signal->rlim[RLIMIT_MEMLOCK].rlim_cur; if (lock_limit == RLIM_INFINITY)

	allowed = 1;
lock_limit >>= PAGE_SHIFT;
spin_lock(&shmlock_user_lock);
if (!allowed &&
    locked + user->locked_shm > lock_limit && !capable(CAP_IPC_LOCK))
	goto out;

... if the user's locked limit (ulimit -l) is set to unlimited, allowed (above) is set to 1. In that case, the second part of that if() is bypassed, and the function grants permission. Therefore, the easy solution is to make sure your user's lock_limit is RLIM_INFINITY.

Hi Ciju:

I am still not exactly sure why this patch is needed. As I read user_shm_lock():

Quoted 9 lines

lock_limit = current->signal->rlim[RLIMIT_MEMLOCK].rlim_cur; if (lock_limit == RLIM_INFINITY)

	allowed = 1;
lock_limit >>= PAGE_SHIFT;
spin_lock(&shmlock_user_lock);
if (!allowed &&
    locked + user->locked_shm > lock_limit && !capable(CAP_IPC_LOCK))
	goto out;

... if the user's locked limit (ulimit -l) is set to unlimited, allowed (above) is set to 1. In that case, the second part of that if() is bypassed, and the function grants permission. Therefore, the easy solution is to make sure your user's lock_limit is RLIM_INFINITY.

On Wed, 2007-11-14 at 19:45 +0530, Ciju Rajan K wrote:
<snip>

Review int user_shm_lock(size_t size, struct us · line 256

Quoted 9 lines

(quoted diff) +4 -0

@@ -248,8 +249,14 @@ int user_shm_lock(size_t size, struct us
         allowed = 1;
     lock_limit >>= PAGE_SHIFT;
     spin_lock(&shmlock_user_lock);
+#ifdef CONFIG_HUGETLB_PAGE
+    if (!allowed &&
+        locked + user->locked_shm > lock_limit &&
+        (!(capable(CAP_IPC_LOCK) || in_group_p(sysctl_hugetlb_shm_group))))
 

This will allow any user in hugetlb_shm_group to make unlimited use of huge page shm segments _and_ normal page shm segments. Definitely not what you want.

Quoted 9 lines

(quoted diff) +2 -0

+#else
     if (!allowed &&
         locked + user->locked_shm > lock_limit && !capable(CAP_IPC_LOCK))
+#endif
         goto out;
     get_uid(user);

     user->locked_shm += locked;

Please don't add new #ifdefs into .c files, headers only.

Quoted 4 lines · aglitke

... if the user's locked limit (ulimit -l) is set to unlimited, allowed (above) is set to 1. In that case, the second part of that if() is bypassed, and the function grants permission. Therefore, the easy solution is to make sure your user's lock_limit is RLIM_INFINITY.

This function deserves a minor cleanup and a bit more commenting.

Reading user->locked_shm within shmlock_user_lock would be nice, too.

Maybe something like this (untested, uncompiled) would do.

-- wli

mm/mlock.c +18 -9

index 7b26560..5f51792 100644
@@ -234,6 +234,12 @@ asmlinkage long sys_munlockall(void)
 /*
  * Objects with different lifetime than processes (SHM_LOCK and SHM_HUGETLB
  * shm segments) get accounted against the user_struct instead.
+ * First, user_shm_lock() checks that the user has permission to lock
+ * enough memory; then if so, the locked shm is accounted to the user's
+ * system-wide state. shmlock_user_lock protects the per-user field
+ * tracking how much locked_shm is in use within the struct user_struct.
+ * shmlock_user_lock is taken early to guard the read-only check that
+ * user->locked_shm is in-bounds against updates to user->locked_shm.
  */
 static DEFINE_SPINLOCK(shmlock_user_lock);
 

@@ -242,19 +248,22 @@ int user_shm_lock(size_t size, struct user_struct *user)
 	unsigned long lock_limit, locked;
 	int allowed = 0;
 
+	spin_lock(&shmlock_user_lock);
 	locked = (size + PAGE_SIZE - 1) >> PAGE_SHIFT;
 	lock_limit = current->signal->rlim[RLIMIT_MEMLOCK].rlim_cur;
 	if (lock_limit == RLIM_INFINITY)
 		allowed = 1;
-	lock_limit >>= PAGE_SHIFT;
-	spin_lock(&shmlock_user_lock);
-	if (!allowed &&
-	    locked + user->locked_shm > lock_limit && !capable(CAP_IPC_LOCK))
-		goto out;
-	get_uid(user);
-	user->locked_shm += locked;
-	allowed = 1;
-out:
+	else {
+		lock_limit >>= PAGE_SHIFT;
+		if (locked + user->locked_shm <= lock_limit)
+			allowed = 1;
+		else if (capable(CAP_IPC_LOCK))
+			allowed = 1;
+	}
+	if (allowed) {
+		get_uid(user);
+		user->locked_shm += locked;
+	}
 	spin_unlock(&shmlock_user_lock);
 	return allowed;
 }

I tested your patch. But that is not solving the problem. If the code change to user_shm_lock() is not a good solution, could
you please suggest a method so that the normal user is able to allocate the huge pages, if his gid is added to /proc/sys/vm/hugetlb_shm_group

Quoted 3 lines · Ciju Rajan K

I tested your patch. But that is not solving the problem.
If the code change to user_shm_lock() is not a good solution, could
you please suggest a method so that the normal user is able to allocate the huge pages, if his gid is added to /proc/sys/vm/hugetlb_shm_group

The patch I posted resolves a race unrelated to your issue. Raising your locked memory limits should not be difficult. /etc/limits.conf or similar should set it up for you. You can also change the default rlimit in the kernel and compile it with default limits elevated to what you want your unprivileged process to have to start with if you're truly having lots of trouble getting userspace to set the default limits properly. I'd look in include/asm-generic/resource.h

-- wli

Quoted 14 lines · William Lee Irwin III

Quoted 3 lines · Ciju Rajan K

I tested your patch. But that is not solving the problem.
If the code change to user_shm_lock() is not a good solution, could
you please suggest a method so that the normal user is able to allocate the huge pages, if his gid is added to /proc/sys/vm/hugetlb_shm_group

The patch I posted resolves a race unrelated to your issue. Raising your locked memory limits should not be difficult. /etc/limits.conf or similar should set it up for you. You can also change the default rlimit in the kernel and compile it with default limits elevated to what you want your unprivileged process to have to start with if you're truly having lots of trouble getting userspace to set the default limits properly. I'd look in include/asm-generic/resource.h

Quoted 1 line

-- wli

Hi Wli,

The documentation available in the kernel for huge pages does not talk about the issue associated with locked memory limit. I think it would be helpful to the users if we mention about this in the documentation. I am attaching a small documentation patch.

Thanks
Ciju

Signed-off-by Ciju Rajan ([email protected])

---
--- Documentation/vm/hugetlbpage.txt.orig 2008-01-24
16:42:40.000000000 +0530
+++ Documentation/vm/hugetlbpage.txt 2008-01-29 19:36:45.000000000 +0530

vm/hugetlbpage.txt 2008-01-29 19:36:45.000000000 +0530 renamed from vm/hugetlbpage.txt.orig 2008-01-24 16:42:40.000000000 +0530 +3 -0

@@ -108,6 +108,18 @@ a supplementary group and system admin n
 applications to use any combination of mmaps and shm* calls, though the
 mount of filesystem will be required for using mmap calls.
 
+Note: The default locked limit in the kernel is just 32KB. If the normal
+user whose gid is present in the file /proc/sys/vm/hugetlb_shm_group needs
+more memory than this, the default limit must be increased. If pam is 

installed
+on your system, resource limits can be configured by installing lines similar

vm/hugetlbpage.txt 2008-01-29 19:36:45.000000000 +0530 renamed from vm/hugetlbpage.txt.orig 2008-01-24 16:42:40.000000000 +0530 +6 -0

+to the following in /etc/security/limits.conf:
+
+@hugegroup      soft    memlock         2097152
+@hugegroup      hard    memlock         2097152
+
+Otherwise, you may manipulate the locked limit command directly with 

'ulimit'.

vm/hugetlbpage.txt 2008-01-29 19:36:45.000000000 +0530 renamed from vm/hugetlbpage.txt.orig 2008-01-24 16:42:40.000000000 +0530 +2 -0

+See its man page for more information.
+
 *******************************************************************
 
 /*

Signature · 5 lines

-- 
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/