[PATCH v2] ipcs: Avoid shmall overflows

[PATCH v2] ipcs: Avoid shmall overflows

[Vasilis Liaskovitis]

Avoid computing the number of bytes in shmall, by only
computing and printing the number of Kbytes. This avoids
some overflows, e.g.

$ echo "4503599627370496" > /proc/sys/kernel/shmall
$ ipcs -l | grep 'max total shared memory'
Before:
max total shared memory (kbytes) = 18014398509481980
After:
max total shared memory (kbytes) = 18014398509481984

$ echo "99993599627370500" > /proc/sys/kernel/shmall
99993599627370500
$ ipcs -l | grep 'max total shared memory'
Before:
max total shared memory (kbytes) = 18014398509481980
After:
max total shared memory (kbytes) = 399974398509482000


v1->v2:
  Print the non-overflow KB value only for IPC_UNIT_KB and
IPC_UNIT_DEFAULT.
  This way --bytes and --human options will still get an expected
output
  (but not avoiding the overflow).

Signed-off-by: Vasilis Liaskovitis <vliaskovitis@suse.com>
---
 sys-utils/ipcs.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/sys-utils/ipcs.c b/sys-utils/ipcs.c
index fc6fba4a6..d36cb1d2a 100644
--- a/sys-utils/ipcs.c
+++ b/sys-utils/ipcs.c
@@ -216,13 +216,19 @@ static void do_shm (char format, int unit)
 		ipc_print_size(unit == IPC_UNIT_DEFAULT ? IPC_UNIT_KB :
unit,
 			       _("max seg size"), lim.shmmax, "\n", 0);
 
-		tmp = (uint64_t) lim.shmall * pgsz;
-		/* overflow handling, at least we don't print
ridiculous small values */
-		if (lim.shmall != 0 && tmp / lim.shmall != pgsz) {
-			tmp = UINT64_MAX - (UINT64_MAX % pgsz);
+		if (unit == IPC_UNIT_KB || unit == IPC_UNIT_DEFAULT) {
+			ipc_print_size(IPC_UNIT_DEFAULT,
+			       _("max total shared memory (kbytes)"),
(pgsz / 1024) *
+			       (uint64_t) lim.shmall, "\n", 0);
+		}
+		else {
+			tmp = (uint64_t) lim.shmall * pgsz;
+			/* overflow handling, at least we don't print
ridiculous small values */
+			if (lim.shmall != 0 && tmp / lim.shmall !=
pgsz) {
+			        tmp = UINT64_MAX - (UINT64_MAX % pgsz);
+		        }
+			ipc_print_size(unit, _("max total shared
memory"), tmp, "\n", 0);
 		}
-		ipc_print_size(unit == IPC_UNIT_DEFAULT ? IPC_UNIT_KB :
unit,
-			       _("max total shared memory"), tmp, "\n",
0);
 		ipc_print_size(unit == IPC_UNIT_DEFAULT ?
IPC_UNIT_BYTES : unit,
 			       _("min seg size"), lim.shmmin, "\n", 0);
 		return;
-- 
2.28.0

Re: [PATCH v2] ipcs: Avoid shmall overflows

[Karel Zak]

On Fri, Jan 15, 2021 at 03:00:11PM +0100, Vasilis Liaskovitis wrote:
>  sys-utils/ipcs.c | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)

Applied, thanks.

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH v2] ipcs: Avoid shmall overflows

[Karel Zak]

On Fri, Jan 15, 2021 at 03:00:11PM +0100, Vasilis Liaskovitis wrote:
>  sys-utils/ipcs.c | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)

...

> +		if (unit == IPC_UNIT_KB || unit == IPC_UNIT_DEFAULT) {
> +			ipc_print_size(IPC_UNIT_DEFAULT,
> +			       _("max total shared memory (kbytes)"), (pgsz / 1024) *
> +			       (uint64_t) lim.shmall, "\n", 0);

I have small disadvantage in this code. There is still possible that
(pgsz / 1024) * lim.shmall is greater than UINT64_MAX. 

It means, we still need to check it. I have added:

   tmp = (uint64_t) lim.shmall * (pgsz / 1024);
   if (lim.shmall != 0 && tmp / lim.shmall != pgsz / 1024)
        tmp = UINT64_MAX - (UINT64_MAX % (pgsz / 1024));

So, now we have separate overflow handling for "kbytes" and for "bytes".

    Karel

> +		}
> +		else {
> +			tmp = (uint64_t) lim.shmall * pgsz;
> +			/* overflow handling, at least we don't print
> ridiculous small values */
> +			if (lim.shmall != 0 && tmp / lim.shmall !=
> pgsz) {
> +			        tmp = UINT64_MAX - (UINT64_MAX % pgsz);
> +		        }
> +			ipc_print_size(unit, _("max total shared
> memory"), tmp, "\n", 0);
>  		}
> -		ipc_print_size(unit == IPC_UNIT_DEFAULT ? IPC_UNIT_KB :
> unit,
> -			       _("max total shared memory"), tmp, "\n",
> 0);
>  		ipc_print_size(unit == IPC_UNIT_DEFAULT ?
> IPC_UNIT_BYTES : unit,
>  			       _("min seg size"), lim.shmmin, "\n", 0);
>  		return;
> -- 
> 2.28.0
> 
> 

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com