linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] ipc/shm.c: increase the limits for SHMMAX, SHMALL
@ 2014-04-19 11:43 Manfred Spraul
  2014-04-19 11:43 ` [PATCH 1/4] ipc/shm.c: check for ulong overflows in shmat Manfred Spraul
  0 siblings, 1 reply; 19+ messages in thread
From: Manfred Spraul @ 2014-04-19 11:43 UTC (permalink / raw)
  To: Davidlohr Bueso, Michael Kerrisk, Martin Schwidefsky
  Cc: LKML, Andrew Morton, KAMEZAWA Hiroyuki, KOSAKI Motohiro, gthelen,
	aswin, linux-mm, Manfred Spraul

Hi all,

the increase of SHMMAX/SHMALL is now a 4 patch series, and still
not ready for merging (see at the end, TASK_SIZE and s390).

If we increase the default limits for SHMMAX and SHMALL,
integer overflows could happen:

SHMMAX:

- shmmem_file_setup places a hard limit on the segment size:
	MAX_LFS_FILESIZE.

	on 32-bit, the limit is > 1 TB.
	--> 32-bit: 4 GB-1 segments are possible.
		Rounded up to full pages the actual allocated size
                is 0.
		--> patch 3

	on 64-bit, this is 0x7fff ffff ffff ffff
		--> no chance for an overflow.

- shmat:
	- find_vma_intersection does not handle overflows properly
		--> patch 1.

	- do_mmap_pgoff limits mappings to TASK_SIZE
		3 GB on 32-bit (assuming x86)
		47 bits on 64-bit (assuming x86)

	- do_mmap_pgoff checks for overflows:
		map 2 GB, starting from addr=2.5GB fails.

SHMALL:

- after creating 8192 segments size (1L<<63)-1, shm_tot
  overflows and returns 0.
	--> patch 2.

And finally:
Patch 4, increase the limits to ULONG_MAX

Open points:
- Better ideas to handle uapi: Is it worth the effort to get
  access to TASK_SIZE? I would say no.
- Better ideas with regards to SHMALL? The values are probably
  large enough, but still arbitrary.

- The TASK_SIZE definition for e.g. S390 differs: It's not
  a constant, instead it is the current task size for current.
  And it seems that the task size can change based on
  (virtual) memory pressure (s390_mmap_check()).
  For new namespaces, this might have interesting effects, i.e.
  this must be fixed.

--
	Manfred

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 1/4] ipc/shm.c: check for ulong overflows in shmat
  2014-04-19 11:43 [PATCH 0/4] ipc/shm.c: increase the limits for SHMMAX, SHMALL Manfred Spraul
@ 2014-04-19 11:43 ` Manfred Spraul
  2014-04-19 11:43   ` [PATCH 2/4] ipc/shm.c: check for overflows of shm_tot Manfred Spraul
  0 siblings, 1 reply; 19+ messages in thread
From: Manfred Spraul @ 2014-04-19 11:43 UTC (permalink / raw)
  To: Davidlohr Bueso, Michael Kerrisk, Martin Schwidefsky
  Cc: LKML, Andrew Morton, KAMEZAWA Hiroyuki, KOSAKI Motohiro, gthelen,
	aswin, linux-mm, Manfred Spraul

find_vma_intersection does not work properly if addr+size overflows.
The patch adds a manual check before the call to find_vma_intersection.

Signed-off-by: Manfred Spraul <manfred@colorfullife.com>
---
 ipc/shm.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/ipc/shm.c b/ipc/shm.c
index 7645961..382e2fb 100644
--- a/ipc/shm.c
+++ b/ipc/shm.c
@@ -1160,6 +1160,9 @@ long do_shmat(int shmid, char __user *shmaddr, int shmflg, ulong *raddr,
 	down_write(&current->mm->mmap_sem);
 	if (addr && !(shmflg & SHM_REMAP)) {
 		err = -EINVAL;
+		if (addr + size < addr)
+			goto invalid;
+
 		if (find_vma_intersection(current->mm, addr, addr + size))
 			goto invalid;
 		/*
-- 
1.9.0

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 2/4] ipc/shm.c: check for overflows of shm_tot
  2014-04-19 11:43 ` [PATCH 1/4] ipc/shm.c: check for ulong overflows in shmat Manfred Spraul
@ 2014-04-19 11:43   ` Manfred Spraul
  2014-04-19 11:43     ` [PATCH 3/4] ipc/shm.c: check for integer overflow during shmget Manfred Spraul
  0 siblings, 1 reply; 19+ messages in thread
From: Manfred Spraul @ 2014-04-19 11:43 UTC (permalink / raw)
  To: Davidlohr Bueso, Michael Kerrisk, Martin Schwidefsky
  Cc: LKML, Andrew Morton, KAMEZAWA Hiroyuki, KOSAKI Motohiro, gthelen,
	aswin, linux-mm, Manfred Spraul

shm_tot counts the total number of pages used by shm segments.

If SHMALL is ULONG_MAX (or nearly ULONG_MAX), then the number
can overflow.  Subsequent calls to shmctl(,SHM_INFO,) would return
wrong values for shm_tot.

The patch adds a detection for overflows.

Signed-off-by: Manfred Spraul <manfred@colorfullife.com>
---
 ipc/shm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ipc/shm.c b/ipc/shm.c
index 382e2fb..2dfa3d6 100644
--- a/ipc/shm.c
+++ b/ipc/shm.c
@@ -493,7 +493,8 @@ static int newseg(struct ipc_namespace *ns, struct ipc_params *params)
 	if (size < SHMMIN || size > ns->shm_ctlmax)
 		return -EINVAL;
 
-	if (ns->shm_tot + numpages > ns->shm_ctlall)
+	if (ns->shm_tot + numpages < ns->shm_tot ||
+			ns->shm_tot + numpages > ns->shm_ctlall)
 		return -ENOSPC;
 
 	shp = ipc_rcu_alloc(sizeof(*shp));
-- 
1.9.0

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 3/4] ipc/shm.c: check for integer overflow during shmget.
  2014-04-19 11:43   ` [PATCH 2/4] ipc/shm.c: check for overflows of shm_tot Manfred Spraul
@ 2014-04-19 11:43     ` Manfred Spraul
  2014-04-19 11:43       ` [PATCH 4/4] ipc/shm.c: Increase the defaults for SHMALL, SHMMAX Manfred Spraul
  0 siblings, 1 reply; 19+ messages in thread
From: Manfred Spraul @ 2014-04-19 11:43 UTC (permalink / raw)
  To: Davidlohr Bueso, Michael Kerrisk, Martin Schwidefsky
  Cc: LKML, Andrew Morton, KAMEZAWA Hiroyuki, KOSAKI Motohiro, gthelen,
	aswin, linux-mm, Manfred Spraul

SHMMAX is the upper limit of a shared memory segment,
counted in bytes. The actual allocation is that size, rounded
up to the next full page.
Add a check that prevents the creation of segments where the
rounded up size causes an integer overflow.

Signed-off-by: Manfred Spraul <manfred@colorfullife.com>
---
 ipc/shm.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/ipc/shm.c b/ipc/shm.c
index 2dfa3d6..f000696 100644
--- a/ipc/shm.c
+++ b/ipc/shm.c
@@ -493,6 +493,9 @@ static int newseg(struct ipc_namespace *ns, struct ipc_params *params)
 	if (size < SHMMIN || size > ns->shm_ctlmax)
 		return -EINVAL;
 
+	if (numpages << PAGE_SHIFT < size)
+		return -ENOSPC;
+
 	if (ns->shm_tot + numpages < ns->shm_tot ||
 			ns->shm_tot + numpages > ns->shm_ctlall)
 		return -ENOSPC;
-- 
1.9.0

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 4/4] ipc/shm.c: Increase the defaults for SHMALL, SHMMAX.
  2014-04-19 11:43     ` [PATCH 3/4] ipc/shm.c: check for integer overflow during shmget Manfred Spraul
@ 2014-04-19 11:43       ` Manfred Spraul
  0 siblings, 0 replies; 19+ messages in thread
From: Manfred Spraul @ 2014-04-19 11:43 UTC (permalink / raw)
  To: Davidlohr Bueso, Michael Kerrisk, Martin Schwidefsky
  Cc: LKML, Andrew Morton, KAMEZAWA Hiroyuki, KOSAKI Motohiro, gthelen,
	aswin, linux-mm, Manfred Spraul

System V shared memory

a) can be abused to trigger out-of-memory conditions and the standard
   measures against out-of-memory do not work:

    - it is not possible to use setrlimit to limit the size of shm segments.

    - segments can exist without association with any processes, thus
      the oom-killer is unable to free that memory.

b) is typically used for shared information - today often multiple GB.
   (e.g. database shared buffers)

The current default is a maximum segment size of 32 MB and a maximum total
size of 8 GB. This is often too much for a) and not enough for b), which
means that lots of users must change the defaults.

This patch increases the default limits to the supported maximum, which is
perfect for case b). The defaults are used after boot and as the initial
value for each new namespace.

Admins/distros that need a protection against a) should reduce the limits
and/or enable shm_rmid_forced.

Further notes:
- The patch only changes default, overrides behave as before:
        # sysctl kernel/shmall=33554432
  would recreate the previous limit for SHMMAX (for the current namespace).

- Disabling sysv shm allocation is possible with:
        # sysctl kernel.shmall=0
  (not a new feature, also per-namespace)

- The limits are intentionally not set to ULONG_MAX, to avoid triggering
  overflows in user space.
  [not unreasonable, see http://marc.info/?l=linux-mm&m=139638334330127]

- The the maximum segment size is set to TASK_SIZE. Segments larger than
  TASK_SIZE do not make sense, because such segments can't be mapped.

- The limit for the total memory is 256*TASK_SIZE.
  This would be 768 GB for x86-32 and 64 PB for x86-64.
  Values larger than that might make sense, but not in the next few weeks.

Signed-off-by: Manfred Spraul <manfred@colorfullife.com>
Reported-by: Davidlohr Bueso <davidlohr@hp.com>
Cc: mtk.manpages@gmail.com
---
 include/linux/shm.h      |  5 ++++-
 include/uapi/linux/shm.h | 10 ++++++++--
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/include/linux/shm.h b/include/linux/shm.h
index 1e2cd2e..7cafb08 100644
--- a/include/linux/shm.h
+++ b/include/linux/shm.h
@@ -4,7 +4,10 @@
 #include <asm/page.h>
 #include <uapi/linux/shm.h>
 
-#define SHMALL (SHMMAX/PAGE_SIZE*(SHMMNI/16)) /* max shm system wide (pages) */
+#define SHMMAX TASK_SIZE		 /* max shared seg size (bytes) */
+#define SHMALL (SHMMAX/PAGE_SIZE*(SHMMNI/16))
+					 /* max shm system wide (pages) */
+
 #include <asm/shmparam.h>
 struct shmid_kernel /* private to the kernel */
 {	
diff --git a/include/uapi/linux/shm.h b/include/uapi/linux/shm.h
index 78b6941..a20bb7a 100644
--- a/include/uapi/linux/shm.h
+++ b/include/uapi/linux/shm.h
@@ -9,14 +9,20 @@
 
 /*
  * SHMMAX, SHMMNI and SHMALL are upper limits are defaults which can
- * be increased by sysctl
+ * be modified by sysctl
  */
 
-#define SHMMAX 0x2000000		 /* max shared seg size (bytes) */
 #define SHMMIN 1			 /* min shared seg size (bytes) */
 #define SHMMNI 4096			 /* max num of segs system wide */
 #ifndef __KERNEL__
+/*
+ * The real values is TASK_SIZE, which is not exported as uapi.
+ * Since this is only the boot time default, 1 GB is a sufficiently
+ * accurate approximation of TASK_SIZE.
+ */
+#define SHMMAX 0x40000000		 /* max shared seg size (bytes) */
 #define SHMALL (SHMMAX/getpagesize()*(SHMMNI/16))
+					 /* max shm system wide (pages) */
 #endif
 #define SHMSEG SHMMNI			 /* max shared segs per process */
 
-- 
1.9.0

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/4] ipc/shm.c: increase the limits for SHMMAX, SHMALL
  2014-09-23  5:24     ` Michael Kerrisk (man-pages)
@ 2014-09-24  8:02       ` Davidlohr Bueso
  0 siblings, 0 replies; 19+ messages in thread
From: Davidlohr Bueso @ 2014-09-24  8:02 UTC (permalink / raw)
  To: Michael Kerrisk (man-pages)
  Cc: Manfred Spraul, Martin Schwidefsky, LKML, Andrew Morton,
	KAMEZAWA Hiroyuki, KOSAKI Motohiro, Greg Thelen, aswin, linux-mm

On Tue, 2014-09-23 at 07:24 +0200, Michael Kerrisk (man-pages) wrote:
> David,
> 
> I applied various pieces from your patch on top of material
> that I already had, so that now we have the text below describing
> these limits.  Comments/suggestions/improvements from all welcome.

Looks good, thanks!

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/4] ipc/shm.c: increase the limits for SHMMAX, SHMALL
  2014-06-03 19:26   ` Davidlohr Bueso
@ 2014-09-23  5:24     ` Michael Kerrisk (man-pages)
  2014-09-24  8:02       ` Davidlohr Bueso
  0 siblings, 1 reply; 19+ messages in thread
From: Michael Kerrisk (man-pages) @ 2014-09-23  5:24 UTC (permalink / raw)
  To: Davidlohr Bueso
  Cc: mtk.manpages, Manfred Spraul, Davidlohr Bueso,
	Martin Schwidefsky, LKML, Andrew Morton, KAMEZAWA Hiroyuki,
	KOSAKI Motohiro, Greg Thelen, aswin, linux-mm

On 06/03/2014 09:26 PM, Davidlohr Bueso wrote:
> On Fri, 2014-05-02 at 15:16 +0200, Michael Kerrisk (man-pages) wrote:
>> Hi Manfred,
>>
>> On Mon, Apr 21, 2014 at 4:26 PM, Manfred Spraul
>> <manfred@colorfullife.com> wrote:
>>> Hi all,
>>>
>>> the increase of SHMMAX/SHMALL is now a 4 patch series.
>>> I don't have ideas how to improve it further.
>>
>> On the assumption that your patches are heading to mainline, could you
>> send me a man-pages patch for the changes?
> 
> It seems we're still behind here and the 3.16 merge window is already
> opened. Please consider this, and again feel free to add/modify as
> necessary. I think adding a note as below is enough and was hesitant to
> add a lot of details... Thanks.
> 
> 8<--------------------------------------------------
> From: Davidlohr Bueso <davidlohr@hp.com>
> Subject: [PATCH] shmget.2: document new limits for shmmax/shmall
> 
> These limits have been recently enlarged and
> modifying them is no longer really necessary.
> Update the manpage.
> 
> Signed-off-by: Davidlohr Bueso <davidlohr@hp.com>
> ---
>  man2/shmget.2 | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/man2/shmget.2 b/man2/shmget.2
> index f781048..77764ea 100644
> --- a/man2/shmget.2
> +++ b/man2/shmget.2
> @@ -299,6 +299,11 @@ with 8kB page size, it yields 2^20 (1048576).
>  
>  On Linux, this limit can be read and modified via
>  .IR /proc/sys/kernel/shmall .
> +As of Linux 3.16, the default value for this limit is increased to
> +.B ULONG_MAX - 2^24
> +pages, which is as large as it can be without helping userspace overflow
> +the values. Modifying this limit is therefore discouraged. This is suitable
> +for both 32 and 64-bit systems.
>  .TP
>  .B SHMMAX
>  Maximum size in bytes for a shared memory segment.
> @@ -306,6 +311,12 @@ Since Linux 2.2, the default value of this limit is 0x2000000 (32MB).
>  
>  On Linux, this limit can be read and modified via
>  .IR /proc/sys/kernel/shmmax .
> +As of Linux 3.16, the default value for this limit is increased from 32Mb
> +to
> +.B ULONG_MAX - 2^24
> +bytes, which is as large as it can be without helping userspace overflow
> +the values. Modifying this limit is therefore discouraged. This is suitable
> +for both 32 and 64-bit systems.
>  .TP
>  .B SHMMIN
>  Minimum size in bytes for a shared memory segment: implementation

David,

I applied various pieces from your patch on top of material
that I already had, so that now we have the text below describing
these limits.  Comments/suggestions/improvements from all welcome.

Cheers,

Michael

       SHMALL System-wide limit on the number of pages of shared memory.

              On  Linux,  this  limit  can  be  read  and  modified  via
              /proc/sys/kernel/shmall.  Since Linux  3.16,  the  default
              value for this limit is:

                  ULONG_MAX - 2^24

              The  effect  of  this  value  (which  is suitable for both
              32-bit and 64-bit systems) is to impose no  limitation  on
              allocations.   This value, rather than ULONG_MAX, was choa??
              sen as the default to prevent some cases where  historical
              applications  simply  raised  the  existing  limit without
              first checking its current value.  Such applications would
              cause  the  value  to  overflow  if  the  limit was set at
              ULONG_MAX.

              From Linux 2.4 up to Linux 3.15,  the  default  value  for
              this limit was:

                  SHMMAX / PAGE_SIZE * (SHMMNI / 16)

              If  SHMMAX  and SHMMNI were not modified, then multiplying
              the result of this formula by the  page  size  (to  get  a
              value  in  bytes)  yielded a value of 8 GB as the limit on
              the total memory used by all shared memory segments.

       SHMMAX Maximum size in bytes for a shared memory segment.

              On  Linux,  this  limit  can  be  read  and  modified  via
              /proc/sys/kernel/shmmax.   Since  Linux  3.16, the default
              value for this limit is:

                  ULONG_MAX - 2^24

              The effect of this  value  (which  is  suitable  for  both
              32-bit  and  64-bit systems) is to impose no limitation on
              allocations.  See the description of SHMALL for a  discusa??
              sion  of why this default value (rather than ULONG_MAX) is
              used.

              From Linux 2.2 up to Linux 3.15, the default value of this
              limit was 0x2000000 (32MB).

              Because  it  is  not possible to map just part of a shared
              memory  segment,  the  amount  of  virtual  memory  places
              another limit on the maximum size of a usable segment: for
              example, on i386 the largest segments that can  be  mapped
              have  a  size of around 2.8 GB, and on x86_64 the limit is
              around 127 TB.



-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/4] ipc/shm.c: increase the limits for SHMMAX, SHMALL
  2014-05-02 13:16 ` Michael Kerrisk (man-pages)
  2014-05-06 20:06   ` Davidlohr Bueso
@ 2014-06-03 19:26   ` Davidlohr Bueso
  2014-09-23  5:24     ` Michael Kerrisk (man-pages)
  1 sibling, 1 reply; 19+ messages in thread
From: Davidlohr Bueso @ 2014-06-03 19:26 UTC (permalink / raw)
  To: mtk.manpages
  Cc: Manfred Spraul, Davidlohr Bueso, Martin Schwidefsky, LKML,
	Andrew Morton, KAMEZAWA Hiroyuki, KOSAKI Motohiro, Greg Thelen,
	aswin, linux-mm

On Fri, 2014-05-02 at 15:16 +0200, Michael Kerrisk (man-pages) wrote:
> Hi Manfred,
> 
> On Mon, Apr 21, 2014 at 4:26 PM, Manfred Spraul
> <manfred@colorfullife.com> wrote:
> > Hi all,
> >
> > the increase of SHMMAX/SHMALL is now a 4 patch series.
> > I don't have ideas how to improve it further.
> 
> On the assumption that your patches are heading to mainline, could you
> send me a man-pages patch for the changes?

It seems we're still behind here and the 3.16 merge window is already
opened. Please consider this, and again feel free to add/modify as
necessary. I think adding a note as below is enough and was hesitant to
add a lot of details... Thanks.

8<--------------------------------------------------
From: Davidlohr Bueso <davidlohr@hp.com>
Subject: [PATCH] shmget.2: document new limits for shmmax/shmall

These limits have been recently enlarged and
modifying them is no longer really necessary.
Update the manpage.

Signed-off-by: Davidlohr Bueso <davidlohr@hp.com>
---
 man2/shmget.2 | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/man2/shmget.2 b/man2/shmget.2
index f781048..77764ea 100644
--- a/man2/shmget.2
+++ b/man2/shmget.2
@@ -299,6 +299,11 @@ with 8kB page size, it yields 2^20 (1048576).
 
 On Linux, this limit can be read and modified via
 .IR /proc/sys/kernel/shmall .
+As of Linux 3.16, the default value for this limit is increased to
+.B ULONG_MAX - 2^24
+pages, which is as large as it can be without helping userspace overflow
+the values. Modifying this limit is therefore discouraged. This is suitable
+for both 32 and 64-bit systems.
 .TP
 .B SHMMAX
 Maximum size in bytes for a shared memory segment.
@@ -306,6 +311,12 @@ Since Linux 2.2, the default value of this limit is 0x2000000 (32MB).
 
 On Linux, this limit can be read and modified via
 .IR /proc/sys/kernel/shmmax .
+As of Linux 3.16, the default value for this limit is increased from 32Mb
+to
+.B ULONG_MAX - 2^24
+bytes, which is as large as it can be without helping userspace overflow
+the values. Modifying this limit is therefore discouraged. This is suitable
+for both 32 and 64-bit systems.
 .TP
 .B SHMMIN
 Minimum size in bytes for a shared memory segment: implementation
-- 
1.8.1.4



--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/4] ipc/shm.c: increase the limits for SHMMAX, SHMALL
  2014-05-07  5:27         ` Michael Kerrisk (man-pages)
@ 2014-05-07 18:22           ` Davidlohr Bueso
  0 siblings, 0 replies; 19+ messages in thread
From: Davidlohr Bueso @ 2014-05-07 18:22 UTC (permalink / raw)
  To: Michael Kerrisk (man-pages)
  Cc: Manfred Spraul, Davidlohr Bueso, Martin Schwidefsky, LKML,
	Andrew Morton, KAMEZAWA Hiroyuki, KOSAKI Motohiro, Greg Thelen,
	aswin, linux-mm

On Wed, 2014-05-07 at 07:27 +0200, Michael Kerrisk (man-pages) wrote:
> On 05/07/2014 12:08 AM, Davidlohr Bueso wrote:
> > On Tue, 2014-05-06 at 22:40 +0200, Michael Kerrisk (man-pages) wrote:
> >> Hi Davidlohr,
> >>
> >> On Tue, May 6, 2014 at 10:06 PM, Davidlohr Bueso <davidlohr@hp.com> wrote:
> >>> On Fri, 2014-05-02 at 15:16 +0200, Michael Kerrisk (man-pages) wrote:
> >>>> Hi Manfred,
> >>>>
> >>>> On Mon, Apr 21, 2014 at 4:26 PM, Manfred Spraul
> >>>> <manfred@colorfullife.com> wrote:
> >>>>> Hi all,
> >>>>>
> >>>>> the increase of SHMMAX/SHMALL is now a 4 patch series.
> >>>>> I don't have ideas how to improve it further.
> >>>>
> >>>> On the assumption that your patches are heading to mainline, could you
> >>>> send me a man-pages patch for the changes?
> >>>
> >>> Btw, I think that the code could still use some love wrt documentation.
> >>
> >> (Agreed.)
> >>
> >>> Andrew, please consider this for -next if folks agree. Thanks.
> >>>
> >>> 8<----------------------------------------------------------
> >>>
> >>> From: Davidlohr Bueso <davidlohr@hp.com>
> >>> Subject: [PATCH] ipc,shm: document new limits in the uapi header
> >>>
> >>> This is useful in the future and allows users to
> >>> better understand the reasoning behind the changes.
> >>>
> >>> Also use UL as we're dealing with it anyways.
> >>>
> >>> Signed-off-by: Davidlohr Bueso <davidlohr@hp.com>
> >>> ---
> >>>  include/uapi/linux/shm.h | 14 ++++++++------
> >>>  1 file changed, 8 insertions(+), 6 deletions(-)
> >>>
> >>> diff --git a/include/uapi/linux/shm.h b/include/uapi/linux/shm.h
> >>> index 74e786d..e37fb08 100644
> >>> --- a/include/uapi/linux/shm.h
> >>> +++ b/include/uapi/linux/shm.h
> >>> @@ -8,17 +8,19 @@
> >>>  #endif
> >>>
> >>>  /*
> >>> - * SHMMAX, SHMMNI and SHMALL are upper limits are defaults which can
> >>
> >> Something is wrong in the line above (missing word(s)?) ("are upper
> >> limits are defaults")
> >>
> >>> - * be modified by sysctl.
> >>> + * SHMMNI, SHMMAX and SHMALL are the default upper limits which can be
> >>> + * modified by sysctl. Both SHMMAX and SHMALL have their default values
> >>> + * to the maximum limit which is as large as it can be without helping
> >>> + * userspace overflow the values. There is really nothing the kernel
> >>> + * can do to avoid this any variables. It is therefore not advised to
> >>
> >> Something is missing in that last line.
> >>
> >>> + * make them any larger. This is suitable for both 32 and 64-bit systems.
> >>
> >> "This" is not so clear. I suggest replacing with an actual noun.
> > 
> > Good point. Perhaps 'These values are ...' would do instead. 
> 
> That's better.
> 
> Did you miss the first point I raised above?

No, actually our emails crossed paths and I had sent a suggestion before
I replied to yours: https://lkml.org/lkml/2014/5/6/613

Thanks.
Davidlohr

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/4] ipc/shm.c: increase the limits for SHMMAX, SHMALL
  2014-05-06 22:08       ` Davidlohr Bueso
@ 2014-05-07  5:27         ` Michael Kerrisk (man-pages)
  2014-05-07 18:22           ` Davidlohr Bueso
  0 siblings, 1 reply; 19+ messages in thread
From: Michael Kerrisk (man-pages) @ 2014-05-07  5:27 UTC (permalink / raw)
  To: Davidlohr Bueso
  Cc: mtk.manpages, Manfred Spraul, Davidlohr Bueso,
	Martin Schwidefsky, LKML, Andrew Morton, KAMEZAWA Hiroyuki,
	KOSAKI Motohiro, Greg Thelen, aswin, linux-mm

On 05/07/2014 12:08 AM, Davidlohr Bueso wrote:
> On Tue, 2014-05-06 at 22:40 +0200, Michael Kerrisk (man-pages) wrote:
>> Hi Davidlohr,
>>
>> On Tue, May 6, 2014 at 10:06 PM, Davidlohr Bueso <davidlohr@hp.com> wrote:
>>> On Fri, 2014-05-02 at 15:16 +0200, Michael Kerrisk (man-pages) wrote:
>>>> Hi Manfred,
>>>>
>>>> On Mon, Apr 21, 2014 at 4:26 PM, Manfred Spraul
>>>> <manfred@colorfullife.com> wrote:
>>>>> Hi all,
>>>>>
>>>>> the increase of SHMMAX/SHMALL is now a 4 patch series.
>>>>> I don't have ideas how to improve it further.
>>>>
>>>> On the assumption that your patches are heading to mainline, could you
>>>> send me a man-pages patch for the changes?
>>>
>>> Btw, I think that the code could still use some love wrt documentation.
>>
>> (Agreed.)
>>
>>> Andrew, please consider this for -next if folks agree. Thanks.
>>>
>>> 8<----------------------------------------------------------
>>>
>>> From: Davidlohr Bueso <davidlohr@hp.com>
>>> Subject: [PATCH] ipc,shm: document new limits in the uapi header
>>>
>>> This is useful in the future and allows users to
>>> better understand the reasoning behind the changes.
>>>
>>> Also use UL as we're dealing with it anyways.
>>>
>>> Signed-off-by: Davidlohr Bueso <davidlohr@hp.com>
>>> ---
>>>  include/uapi/linux/shm.h | 14 ++++++++------
>>>  1 file changed, 8 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/include/uapi/linux/shm.h b/include/uapi/linux/shm.h
>>> index 74e786d..e37fb08 100644
>>> --- a/include/uapi/linux/shm.h
>>> +++ b/include/uapi/linux/shm.h
>>> @@ -8,17 +8,19 @@
>>>  #endif
>>>
>>>  /*
>>> - * SHMMAX, SHMMNI and SHMALL are upper limits are defaults which can
>>
>> Something is wrong in the line above (missing word(s)?) ("are upper
>> limits are defaults")
>>
>>> - * be modified by sysctl.
>>> + * SHMMNI, SHMMAX and SHMALL are the default upper limits which can be
>>> + * modified by sysctl. Both SHMMAX and SHMALL have their default values
>>> + * to the maximum limit which is as large as it can be without helping
>>> + * userspace overflow the values. There is really nothing the kernel
>>> + * can do to avoid this any variables. It is therefore not advised to
>>
>> Something is missing in that last line.
>>
>>> + * make them any larger. This is suitable for both 32 and 64-bit systems.
>>
>> "This" is not so clear. I suggest replacing with an actual noun.
> 
> Good point. Perhaps 'These values are ...' would do instead. 

That's better.

Did you miss the first point I raised above?

Cheers,

Michael


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/4] ipc/shm.c: increase the limits for SHMMAX, SHMALL
  2014-05-06 20:40     ` Michael Kerrisk (man-pages)
@ 2014-05-06 22:08       ` Davidlohr Bueso
  2014-05-07  5:27         ` Michael Kerrisk (man-pages)
  0 siblings, 1 reply; 19+ messages in thread
From: Davidlohr Bueso @ 2014-05-06 22:08 UTC (permalink / raw)
  To: mtk.manpages
  Cc: Manfred Spraul, Davidlohr Bueso, Martin Schwidefsky, LKML,
	Andrew Morton, KAMEZAWA Hiroyuki, KOSAKI Motohiro, Greg Thelen,
	aswin, linux-mm

On Tue, 2014-05-06 at 22:40 +0200, Michael Kerrisk (man-pages) wrote:
> Hi Davidlohr,
> 
> On Tue, May 6, 2014 at 10:06 PM, Davidlohr Bueso <davidlohr@hp.com> wrote:
> > On Fri, 2014-05-02 at 15:16 +0200, Michael Kerrisk (man-pages) wrote:
> >> Hi Manfred,
> >>
> >> On Mon, Apr 21, 2014 at 4:26 PM, Manfred Spraul
> >> <manfred@colorfullife.com> wrote:
> >> > Hi all,
> >> >
> >> > the increase of SHMMAX/SHMALL is now a 4 patch series.
> >> > I don't have ideas how to improve it further.
> >>
> >> On the assumption that your patches are heading to mainline, could you
> >> send me a man-pages patch for the changes?
> >
> > Btw, I think that the code could still use some love wrt documentation.
> 
> (Agreed.)
> 
> > Andrew, please consider this for -next if folks agree. Thanks.
> >
> > 8<----------------------------------------------------------
> >
> > From: Davidlohr Bueso <davidlohr@hp.com>
> > Subject: [PATCH] ipc,shm: document new limits in the uapi header
> >
> > This is useful in the future and allows users to
> > better understand the reasoning behind the changes.
> >
> > Also use UL as we're dealing with it anyways.
> >
> > Signed-off-by: Davidlohr Bueso <davidlohr@hp.com>
> > ---
> >  include/uapi/linux/shm.h | 14 ++++++++------
> >  1 file changed, 8 insertions(+), 6 deletions(-)
> >
> > diff --git a/include/uapi/linux/shm.h b/include/uapi/linux/shm.h
> > index 74e786d..e37fb08 100644
> > --- a/include/uapi/linux/shm.h
> > +++ b/include/uapi/linux/shm.h
> > @@ -8,17 +8,19 @@
> >  #endif
> >
> >  /*
> > - * SHMMAX, SHMMNI and SHMALL are upper limits are defaults which can
> 
> Something is wrong in the line above (missing word(s)?) ("are upper
> limits are defaults")
> 
> > - * be modified by sysctl.
> > + * SHMMNI, SHMMAX and SHMALL are the default upper limits which can be
> > + * modified by sysctl. Both SHMMAX and SHMALL have their default values
> > + * to the maximum limit which is as large as it can be without helping
> > + * userspace overflow the values. There is really nothing the kernel
> > + * can do to avoid this any variables. It is therefore not advised to
> 
> Something is missing in that last line.
> 
> > + * make them any larger. This is suitable for both 32 and 64-bit systems.
> 
> "This" is not so clear. I suggest replacing with an actual noun.

Good point. Perhaps 'These values are ...' would do instead. 

Thanks,
Davidlohr

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/4] ipc/shm.c: increase the limits for SHMMAX, SHMALL
  2014-05-06 20:06   ` Davidlohr Bueso
  2014-05-06 20:40     ` Michael Kerrisk (man-pages)
@ 2014-05-06 20:43     ` Davidlohr Bueso
  1 sibling, 0 replies; 19+ messages in thread
From: Davidlohr Bueso @ 2014-05-06 20:43 UTC (permalink / raw)
  To: mtk.manpages
  Cc: Manfred Spraul, Davidlohr Bueso, Martin Schwidefsky, LKML,
	Andrew Morton, KAMEZAWA Hiroyuki, KOSAKI Motohiro, Greg Thelen,
	aswin, linux-mm

On Tue, 2014-05-06 at 13:06 -0700, Davidlohr Bueso wrote:
> On Fri, 2014-05-02 at 15:16 +0200, Michael Kerrisk (man-pages) wrote:
> > Hi Manfred,
> > 
> > On Mon, Apr 21, 2014 at 4:26 PM, Manfred Spraul
> > <manfred@colorfullife.com> wrote:
> > > Hi all,
> > >
> > > the increase of SHMMAX/SHMALL is now a 4 patch series.
> > > I don't have ideas how to improve it further.
> > 
> > On the assumption that your patches are heading to mainline, could you
> > send me a man-pages patch for the changes?
> 
> Btw, I think that the code could still use some love wrt documentation.
> Andrew, please consider this for -next if folks agree. Thanks.
> 
> 8<----------------------------------------------------------
> 
> From: Davidlohr Bueso <davidlohr@hp.com>
> Subject: [PATCH] ipc,shm: document new limits in the uapi header
> 
> This is useful in the future and allows users to
> better understand the reasoning behind the changes.
> 
> Also use UL as we're dealing with it anyways.
> 
> Signed-off-by: Davidlohr Bueso <davidlohr@hp.com>
> ---
>  include/uapi/linux/shm.h | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/include/uapi/linux/shm.h b/include/uapi/linux/shm.h
> index 74e786d..e37fb08 100644
> --- a/include/uapi/linux/shm.h
> +++ b/include/uapi/linux/shm.h
> @@ -8,17 +8,19 @@
>  #endif
>  
>  /*
> - * SHMMAX, SHMMNI and SHMALL are upper limits are defaults which can
> - * be modified by sysctl.
> + * SHMMNI, SHMMAX and SHMALL are the default upper limits which can be
> + * modified by sysctl. Both SHMMAX and SHMALL have their default values
> + * to the maximum limit which is as large as it can be without helping
> + * userspace overflow the values. There is really nothing the kernel
> + * can do to avoid this any variables. It is therefore not advised to
                              ^^^^^^^^^^ should be 'further', sorry.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/4] ipc/shm.c: increase the limits for SHMMAX, SHMALL
  2014-05-06 20:06   ` Davidlohr Bueso
@ 2014-05-06 20:40     ` Michael Kerrisk (man-pages)
  2014-05-06 22:08       ` Davidlohr Bueso
  2014-05-06 20:43     ` Davidlohr Bueso
  1 sibling, 1 reply; 19+ messages in thread
From: Michael Kerrisk (man-pages) @ 2014-05-06 20:40 UTC (permalink / raw)
  To: Davidlohr Bueso
  Cc: Manfred Spraul, Davidlohr Bueso, Martin Schwidefsky, LKML,
	Andrew Morton, KAMEZAWA Hiroyuki, KOSAKI Motohiro, Greg Thelen,
	aswin, linux-mm

Hi Davidlohr,

On Tue, May 6, 2014 at 10:06 PM, Davidlohr Bueso <davidlohr@hp.com> wrote:
> On Fri, 2014-05-02 at 15:16 +0200, Michael Kerrisk (man-pages) wrote:
>> Hi Manfred,
>>
>> On Mon, Apr 21, 2014 at 4:26 PM, Manfred Spraul
>> <manfred@colorfullife.com> wrote:
>> > Hi all,
>> >
>> > the increase of SHMMAX/SHMALL is now a 4 patch series.
>> > I don't have ideas how to improve it further.
>>
>> On the assumption that your patches are heading to mainline, could you
>> send me a man-pages patch for the changes?
>
> Btw, I think that the code could still use some love wrt documentation.

(Agreed.)

> Andrew, please consider this for -next if folks agree. Thanks.
>
> 8<----------------------------------------------------------
>
> From: Davidlohr Bueso <davidlohr@hp.com>
> Subject: [PATCH] ipc,shm: document new limits in the uapi header
>
> This is useful in the future and allows users to
> better understand the reasoning behind the changes.
>
> Also use UL as we're dealing with it anyways.
>
> Signed-off-by: Davidlohr Bueso <davidlohr@hp.com>
> ---
>  include/uapi/linux/shm.h | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/include/uapi/linux/shm.h b/include/uapi/linux/shm.h
> index 74e786d..e37fb08 100644
> --- a/include/uapi/linux/shm.h
> +++ b/include/uapi/linux/shm.h
> @@ -8,17 +8,19 @@
>  #endif
>
>  /*
> - * SHMMAX, SHMMNI and SHMALL are upper limits are defaults which can

Something is wrong in the line above (missing word(s)?) ("are upper
limits are defaults")

> - * be modified by sysctl.
> + * SHMMNI, SHMMAX and SHMALL are the default upper limits which can be
> + * modified by sysctl. Both SHMMAX and SHMALL have their default values
> + * to the maximum limit which is as large as it can be without helping
> + * userspace overflow the values. There is really nothing the kernel
> + * can do to avoid this any variables. It is therefore not advised to

Something is missing in that last line.

> + * make them any larger. This is suitable for both 32 and 64-bit systems.

"This" is not so clear. I suggest replacing with an actual noun.

>   */
> -
>  #define SHMMIN 1                        /* min shared seg size (bytes) */
>  #define SHMMNI 4096                     /* max num of segs system wide */
> -#define SHMMAX (ULONG_MAX - (1L<<24))   /* max shared seg size (bytes) */
> -#define SHMALL (ULONG_MAX - (1L<<24))   /* max shm system wide (pages) */
> +#define SHMMAX (ULONG_MAX - (1UL << 24)) /* max shared seg size (bytes) */
> +#define SHMALL (ULONG_MAX - (1UL << 24)) /* max shm system wide (pages) */
>  #define SHMSEG SHMMNI                   /* max shared segs per process */
>
> -
>  /* Obsolete, used only for backwards compatibility and libc5 compiles */
>  struct shmid_ds {
>         struct ipc_perm         shm_perm;       /* operation perms */

Cheers,

Michael

-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/4] ipc/shm.c: increase the limits for SHMMAX, SHMALL
  2014-05-02 13:16 ` Michael Kerrisk (man-pages)
@ 2014-05-06 20:06   ` Davidlohr Bueso
  2014-05-06 20:40     ` Michael Kerrisk (man-pages)
  2014-05-06 20:43     ` Davidlohr Bueso
  2014-06-03 19:26   ` Davidlohr Bueso
  1 sibling, 2 replies; 19+ messages in thread
From: Davidlohr Bueso @ 2014-05-06 20:06 UTC (permalink / raw)
  To: mtk.manpages
  Cc: Manfred Spraul, Davidlohr Bueso, Martin Schwidefsky, LKML,
	Andrew Morton, KAMEZAWA Hiroyuki, KOSAKI Motohiro, Greg Thelen,
	aswin, linux-mm

On Fri, 2014-05-02 at 15:16 +0200, Michael Kerrisk (man-pages) wrote:
> Hi Manfred,
> 
> On Mon, Apr 21, 2014 at 4:26 PM, Manfred Spraul
> <manfred@colorfullife.com> wrote:
> > Hi all,
> >
> > the increase of SHMMAX/SHMALL is now a 4 patch series.
> > I don't have ideas how to improve it further.
> 
> On the assumption that your patches are heading to mainline, could you
> send me a man-pages patch for the changes?

Btw, I think that the code could still use some love wrt documentation.
Andrew, please consider this for -next if folks agree. Thanks.

8<----------------------------------------------------------

From: Davidlohr Bueso <davidlohr@hp.com>
Subject: [PATCH] ipc,shm: document new limits in the uapi header

This is useful in the future and allows users to
better understand the reasoning behind the changes.

Also use UL as we're dealing with it anyways.

Signed-off-by: Davidlohr Bueso <davidlohr@hp.com>
---
 include/uapi/linux/shm.h | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/include/uapi/linux/shm.h b/include/uapi/linux/shm.h
index 74e786d..e37fb08 100644
--- a/include/uapi/linux/shm.h
+++ b/include/uapi/linux/shm.h
@@ -8,17 +8,19 @@
 #endif
 
 /*
- * SHMMAX, SHMMNI and SHMALL are upper limits are defaults which can
- * be modified by sysctl.
+ * SHMMNI, SHMMAX and SHMALL are the default upper limits which can be
+ * modified by sysctl. Both SHMMAX and SHMALL have their default values
+ * to the maximum limit which is as large as it can be without helping
+ * userspace overflow the values. There is really nothing the kernel
+ * can do to avoid this any variables. It is therefore not advised to
+ * make them any larger. This is suitable for both 32 and 64-bit systems.
  */
-
 #define SHMMIN 1			 /* min shared seg size (bytes) */
 #define SHMMNI 4096			 /* max num of segs system wide */
-#define SHMMAX (ULONG_MAX - (1L<<24))	 /* max shared seg size (bytes) */
-#define SHMALL (ULONG_MAX - (1L<<24))	 /* max shm system wide (pages) */
+#define SHMMAX (ULONG_MAX - (1UL << 24)) /* max shared seg size (bytes) */
+#define SHMALL (ULONG_MAX - (1UL << 24)) /* max shm system wide (pages) */
 #define SHMSEG SHMMNI			 /* max shared segs per process */
 
-
 /* Obsolete, used only for backwards compatibility and libc5 compiles */
 struct shmid_ds {
 	struct ipc_perm		shm_perm;	/* operation perms */
-- 
1.8.1.4



--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/4] ipc/shm.c: increase the limits for SHMMAX, SHMALL
  2014-04-21 14:26 [PATCH 0/4] ipc/shm.c: increase the limits for SHMMAX, SHMALL Manfred Spraul
  2014-04-21 17:25 ` Davidlohr Bueso
@ 2014-05-02 13:16 ` Michael Kerrisk (man-pages)
  2014-05-06 20:06   ` Davidlohr Bueso
  2014-06-03 19:26   ` Davidlohr Bueso
  1 sibling, 2 replies; 19+ messages in thread
From: Michael Kerrisk (man-pages) @ 2014-05-02 13:16 UTC (permalink / raw)
  To: Manfred Spraul
  Cc: Davidlohr Bueso, Martin Schwidefsky, LKML, Andrew Morton,
	KAMEZAWA Hiroyuki, KOSAKI Motohiro, Greg Thelen, aswin, linux-mm

Hi Manfred,

On Mon, Apr 21, 2014 at 4:26 PM, Manfred Spraul
<manfred@colorfullife.com> wrote:
> Hi all,
>
> the increase of SHMMAX/SHMALL is now a 4 patch series.
> I don't have ideas how to improve it further.

On the assumption that your patches are heading to mainline, could you
send me a man-pages patch for the changes?

Thanks,

Michael


> The change itself is trivial, the only problem are interger overflows.
> The overflows are not new, but if we make huge values the default,
> then the code should be free from overflows.
>
> SHMMAX:
>
> - shmmem_file_setup places a hard limit on the segment size:
>   MAX_LFS_FILESIZE.
>
>   On 32-bit, the limit is > 1 TB, i.e. 4 GB-1 byte segments are
>   possible. Rounded up to full pages the actual allocated size
>   is 0. --> must be fixed, patch 3
>
> - shmat:
>   - find_vma_intersection does not handle overflows properly.
>     --> must be fixed, patch 1
>
>   - the rest is fine, do_mmap_pgoff limits mappings to TASK_SIZE
>     and checks for overflows (i.e.: map 2 GB, starting from
>     addr=2.5GB fails).
>
> SHMALL:
> - after creating 8192 segments size (1L<<63)-1, shm_tot overflows and
>   returns 0.  --> must be fixed, patch 2.
>
> User space:
> - Obviuosly, there could be overflows in user space. There is nothing
>   we can do, only use values smaller than ULONG_MAX.
>   I ended with "ULONG_MAX - 1L<<24":
>
>   - TASK_SIZE cannot be used because it is the size of the current
>     task. Could be 4G if it's a 32-bit task on a 64-bit kernel.
>
>   - The maximum size is not standardized across archs:
>     I found TASK_MAX_SIZE, TASK_SIZE_MAX and TASK_SIZE_64.
>
>   - Just in case some arch revives a 4G/4G split, nearly
>     ULONG_MAX is a valid segment size.
>
>   - Using "0" as a magic value for infinity is even worse, because
>     right now 0 means 0, i.e. fail all allocations.
>
> Andrew: Could you add it into -akpm and move it towards linux-next?
>
> --
>         Manfred



-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/4] ipc/shm.c: increase the limits for SHMMAX, SHMALL
  2014-04-22  4:23   ` Manfred Spraul
@ 2014-04-22 18:18     ` Davidlohr Bueso
  0 siblings, 0 replies; 19+ messages in thread
From: Davidlohr Bueso @ 2014-04-22 18:18 UTC (permalink / raw)
  To: Manfred Spraul
  Cc: Davidlohr Bueso, Michael Kerrisk, Martin Schwidefsky, LKML,
	Andrew Morton, KAMEZAWA Hiroyuki, KOSAKI Motohiro, gthelen,
	aswin, linux-mm

On Tue, 2014-04-22 at 06:23 +0200, Manfred Spraul wrote:
> On 04/21/2014 07:25 PM, Davidlohr Bueso wrote:
> > On Mon, 2014-04-21 at 16:26 +0200, Manfred Spraul wrote:
> >> Hi all,
> >>
> >> the increase of SHMMAX/SHMALL is now a 4 patch series.
> >> I don't have ideas how to improve it further.
> > Manfred, is there any difference between this set and the one you sent a
> > couple of days ago?
> a) I updated the comments.
> b) the initial set used TASK_SIZE, not I switch to ULONG_MAX-(1L<<24)
> 
> >>    - Using "0" as a magic value for infinity is even worse, because
> >>      right now 0 means 0, i.e. fail all allocations.
> > Sorry but I don't quite get this. Using 0 eliminates the need for all
> > these patches, no? I mean overflows have existed since forever, and
> > taking this route would naturally solve the problem. 0 allocations are a
> > no no anyways.
> No. The patches are required to handle e.g. shmget(,ULONG_MAX,):
> Right now, shmget(,ULONG_MAX,) results in a 0-byte segment.

Ok, I was mixing 'issues' then.

> The risk of using 0 is that it reverses the current behavior:
> Up to now,
>      # sysctl kernel.shmall=0
> disables allocations.
> If we define 0 a infinity, then the same configuration would allow 
> unlimited allocations.

Right, but as I mentioned, this also contradicts the fact that shmmin
cannot be 0. And again, I don't know who's correct here. Do any
standards mention this? I haven't found anything, and hard-codding
shmmin to 1 seems to be different among OSs, Linux choosing to do so.
This difference must also be commented in the manpage.

That said, I believe that violating this "feature" and forbidding
disabling shm would probably have a more severe penalty (security,
perhaps) for users who rely on this. So while I'm really annoyed that we
"cannot" use 0 because of this, I'm going to give up arguing. I believe
you approach is the safer way of going.

Thanks a lot for looking into this, Manfred.
Davidlohr

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/4] ipc/shm.c: increase the limits for SHMMAX, SHMALL
  2014-04-21 17:25 ` Davidlohr Bueso
@ 2014-04-22  4:23   ` Manfred Spraul
  2014-04-22 18:18     ` Davidlohr Bueso
  0 siblings, 1 reply; 19+ messages in thread
From: Manfred Spraul @ 2014-04-22  4:23 UTC (permalink / raw)
  To: Davidlohr Bueso
  Cc: Davidlohr Bueso, Michael Kerrisk, Martin Schwidefsky, LKML,
	Andrew Morton, KAMEZAWA Hiroyuki, KOSAKI Motohiro, gthelen,
	aswin, linux-mm

On 04/21/2014 07:25 PM, Davidlohr Bueso wrote:
> On Mon, 2014-04-21 at 16:26 +0200, Manfred Spraul wrote:
>> Hi all,
>>
>> the increase of SHMMAX/SHMALL is now a 4 patch series.
>> I don't have ideas how to improve it further.
> Manfred, is there any difference between this set and the one you sent a
> couple of days ago?
a) I updated the comments.
b) the initial set used TASK_SIZE, not I switch to ULONG_MAX-(1L<<24)

>>    - Using "0" as a magic value for infinity is even worse, because
>>      right now 0 means 0, i.e. fail all allocations.
> Sorry but I don't quite get this. Using 0 eliminates the need for all
> these patches, no? I mean overflows have existed since forever, and
> taking this route would naturally solve the problem. 0 allocations are a
> no no anyways.
No. The patches are required to handle e.g. shmget(,ULONG_MAX,):
Right now, shmget(,ULONG_MAX,) results in a 0-byte segment.

The risk of using 0 is that it reverses the current behavior:
Up to now,
     # sysctl kernel.shmall=0
disables allocations.
If we define 0 a infinity, then the same configuration would allow 
unlimited allocations.

--
     Manfred

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/4] ipc/shm.c: increase the limits for SHMMAX, SHMALL
  2014-04-21 14:26 [PATCH 0/4] ipc/shm.c: increase the limits for SHMMAX, SHMALL Manfred Spraul
@ 2014-04-21 17:25 ` Davidlohr Bueso
  2014-04-22  4:23   ` Manfred Spraul
  2014-05-02 13:16 ` Michael Kerrisk (man-pages)
  1 sibling, 1 reply; 19+ messages in thread
From: Davidlohr Bueso @ 2014-04-21 17:25 UTC (permalink / raw)
  To: Manfred Spraul
  Cc: Davidlohr Bueso, Michael Kerrisk, Martin Schwidefsky, LKML,
	Andrew Morton, KAMEZAWA Hiroyuki, KOSAKI Motohiro, gthelen,
	aswin, linux-mm

On Mon, 2014-04-21 at 16:26 +0200, Manfred Spraul wrote:
> Hi all,
> 
> the increase of SHMMAX/SHMALL is now a 4 patch series.
> I don't have ideas how to improve it further.

Manfred, is there any difference between this set and the one you sent a
couple of days ago?

> 
> The change itself is trivial, the only problem are interger overflows.
> The overflows are not new, but if we make huge values the default,
> then the code should be free from overflows.
> 
> SHMMAX:
> 
> - shmmem_file_setup places a hard limit on the segment size:
>   MAX_LFS_FILESIZE.
> 
>   On 32-bit, the limit is > 1 TB, i.e. 4 GB-1 byte segments are
>   possible. Rounded up to full pages the actual allocated size
>   is 0. --> must be fixed, patch 3
> 
> - shmat:
>   - find_vma_intersection does not handle overflows properly.
>     --> must be fixed, patch 1
> 
>   - the rest is fine, do_mmap_pgoff limits mappings to TASK_SIZE
>     and checks for overflows (i.e.: map 2 GB, starting from
>     addr=2.5GB fails).
> 
> SHMALL:
> - after creating 8192 segments size (1L<<63)-1, shm_tot overflows and
>   returns 0.  --> must be fixed, patch 2.
> 
> User space:
> - Obviuosly, there could be overflows in user space. There is nothing
>   we can do, only use values smaller than ULONG_MAX.
>   I ended with "ULONG_MAX - 1L<<24":
> 
>   - TASK_SIZE cannot be used because it is the size of the current
>     task. Could be 4G if it's a 32-bit task on a 64-bit kernel.
> 
>   - The maximum size is not standardized across archs:
>     I found TASK_MAX_SIZE, TASK_SIZE_MAX and TASK_SIZE_64.
> 
>   - Just in case some arch revives a 4G/4G split, nearly
>     ULONG_MAX is a valid segment size.
> 
>   - Using "0" as a magic value for infinity is even worse, because
>     right now 0 means 0, i.e. fail all allocations.

Sorry but I don't quite get this. Using 0 eliminates the need for all
these patches, no? I mean overflows have existed since forever, and
taking this route would naturally solve the problem. 0 allocations are a
no no anyways.

I do agree with the series iff we endup taking this 'increase the limit
size approach'. But I just don't see the need.

Thanks,
Davidlohr

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 0/4] ipc/shm.c: increase the limits for SHMMAX, SHMALL
@ 2014-04-21 14:26 Manfred Spraul
  2014-04-21 17:25 ` Davidlohr Bueso
  2014-05-02 13:16 ` Michael Kerrisk (man-pages)
  0 siblings, 2 replies; 19+ messages in thread
From: Manfred Spraul @ 2014-04-21 14:26 UTC (permalink / raw)
  To: Davidlohr Bueso, Michael Kerrisk, Martin Schwidefsky
  Cc: LKML, Andrew Morton, KAMEZAWA Hiroyuki, KOSAKI Motohiro, gthelen,
	aswin, linux-mm, Manfred Spraul

Hi all,

the increase of SHMMAX/SHMALL is now a 4 patch series.
I don't have ideas how to improve it further.

The change itself is trivial, the only problem are interger overflows.
The overflows are not new, but if we make huge values the default,
then the code should be free from overflows.

SHMMAX:

- shmmem_file_setup places a hard limit on the segment size:
  MAX_LFS_FILESIZE.

  On 32-bit, the limit is > 1 TB, i.e. 4 GB-1 byte segments are
  possible. Rounded up to full pages the actual allocated size
  is 0. --> must be fixed, patch 3

- shmat:
  - find_vma_intersection does not handle overflows properly.
    --> must be fixed, patch 1

  - the rest is fine, do_mmap_pgoff limits mappings to TASK_SIZE
    and checks for overflows (i.e.: map 2 GB, starting from
    addr=2.5GB fails).

SHMALL:
- after creating 8192 segments size (1L<<63)-1, shm_tot overflows and
  returns 0.  --> must be fixed, patch 2.

User space:
- Obviuosly, there could be overflows in user space. There is nothing
  we can do, only use values smaller than ULONG_MAX.
  I ended with "ULONG_MAX - 1L<<24":

  - TASK_SIZE cannot be used because it is the size of the current
    task. Could be 4G if it's a 32-bit task on a 64-bit kernel.

  - The maximum size is not standardized across archs:
    I found TASK_MAX_SIZE, TASK_SIZE_MAX and TASK_SIZE_64.

  - Just in case some arch revives a 4G/4G split, nearly
    ULONG_MAX is a valid segment size.

  - Using "0" as a magic value for infinity is even worse, because
    right now 0 means 0, i.e. fail all allocations.

Andrew: Could you add it into -akpm and move it towards linux-next?

--
	Manfred

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2014-09-24  8:02 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-19 11:43 [PATCH 0/4] ipc/shm.c: increase the limits for SHMMAX, SHMALL Manfred Spraul
2014-04-19 11:43 ` [PATCH 1/4] ipc/shm.c: check for ulong overflows in shmat Manfred Spraul
2014-04-19 11:43   ` [PATCH 2/4] ipc/shm.c: check for overflows of shm_tot Manfred Spraul
2014-04-19 11:43     ` [PATCH 3/4] ipc/shm.c: check for integer overflow during shmget Manfred Spraul
2014-04-19 11:43       ` [PATCH 4/4] ipc/shm.c: Increase the defaults for SHMALL, SHMMAX Manfred Spraul
2014-04-21 14:26 [PATCH 0/4] ipc/shm.c: increase the limits for SHMMAX, SHMALL Manfred Spraul
2014-04-21 17:25 ` Davidlohr Bueso
2014-04-22  4:23   ` Manfred Spraul
2014-04-22 18:18     ` Davidlohr Bueso
2014-05-02 13:16 ` Michael Kerrisk (man-pages)
2014-05-06 20:06   ` Davidlohr Bueso
2014-05-06 20:40     ` Michael Kerrisk (man-pages)
2014-05-06 22:08       ` Davidlohr Bueso
2014-05-07  5:27         ` Michael Kerrisk (man-pages)
2014-05-07 18:22           ` Davidlohr Bueso
2014-05-06 20:43     ` Davidlohr Bueso
2014-06-03 19:26   ` Davidlohr Bueso
2014-09-23  5:24     ` Michael Kerrisk (man-pages)
2014-09-24  8:02       ` Davidlohr Bueso

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox