On 04/11/2014 10:27 PM, Davidlohr Bueso wrote:
On Fri, 2014-04-11 at 20:28 +0200, Manfred Spraul wrote:
Hi Davidlohr,
On 04/03/2014 02:20 AM, Davidlohr Bueso wrote:
The default size for shmmax is, and always has been, 32Mb.
Today, in the XXI century, it seems that this value is rather small,
making users have to increase it via sysctl, which can cause
unnecessary work and userspace application workarounds[1].
[snip]
Running this patch through LTP, everything passes, except the following,
which, due to the nature of this change, is quite expected:
shmget02 1 TFAIL : call succeeded unexpectedly
Why is this TFAIL expected?
So looking at shmget02.c, this is the case that fails:
for (i = 0; i < TST_TOTAL; i++) {
/*
* Look for a failure ...
*/
TEST(shmget(*(TC[i].skey), TC[i].size, TC[i].flags));
if (TEST_RETURN != -1) {
tst_resm(TFAIL, "call succeeded unexpectedly");
continue;
}
Where TC[0] is:
struct test_case_t {
int *skey;
int size;
int flags;
int error;
} TC[] = {
/* EINVAL - size is 0 */
{
&shmkey2, 0, IPC_CREAT | IPC_EXCL | SHM_RW, EINVAL},
So it's expected because now 0 is actually valid. And before:
EINVAL A new segment was to be created and size < SHMMIN or size > SHMMAX
diff --git a/ipc/shm.c b/ipc/shm.c
index 7645961..ae01ffa 100644
--- a/ipc/shm.c
+++ b/ipc/shm.c
@@ -490,10 +490,12 @@ static int newseg(struct ipc_namespace *ns, struct ipc_params *params)
int id;
vm_flags_t acctflag = 0;
- if (size < SHMMIN || size > ns->shm_ctlmax)
+ if (ns->shm_ctlmax &&
+ (size < SHMMIN || size > ns->shm_ctlmax))
return -EINVAL;
- if (ns->shm_tot + numpages > ns->shm_ctlall)
+ if (ns->shm_ctlall &&
+ ns->shm_tot + numpages > ns->shm_ctlall)
return -ENOSPC;
shp = ipc_rcu_alloc(sizeof(*shp));
Ok, I understand it:
Your patch disables checking shmmax, shmall *AND* checking for SHMMIN.
Right, if shmmax is 0, then there's no point checking for shmmin,
otherwise we'd always end up returning EINVAL.
a) Have you double checked that 0-sized shm segments work properly?
Does the swap code handle it properly, ...? EINVAL A new segment was to be created and size < SHMMIN or size > SHMMAX
Hmm so I've been using this patch just fine on my laptop since I sent
it. So far I haven't seen any issues. Are you refering to something in
particular? I'd be happy to run any cases you're concerned with.
I'm thinking about malicious applications.
Create 0-sized segments and then map them. Does find_vma_intersection
handle that case?
The same for all other functions that are called by the shm code.
You can't replace code review by "runs for a month"
b) It's that yet another risk for user space incompatibility?
Sorry, I don't follow here.
Applications expect that shmget(,0,) fails.
--
Manfred
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>