linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [patch] x86/ldt: make a size variable unsigned
@ 2016-12-08 10:56 Dan Carpenter
  2016-12-08 22:29 ` Andy Lutomirski
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Dan Carpenter @ 2016-12-08 10:56 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Ingo Molnar, H. Peter Anvin, x86, Dave Hansen, Andrew Morton,
	Andrey Ryabinin, Andy Lutomirski, linux-kernel, kernel-janitors

My static checker complains that we put an upper bound on the "size"
variable but not a lower bound.  The checker is not smart enough to know
the possible ranges of "old_mm->context.ldt->size" from
init_new_context_ldt() so it thinks maybe it could be negative.

Let's make it unsigned to silence the warning and future proof the code
a bit.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/arch/x86/kernel/ldt.c b/arch/x86/kernel/ldt.c
index 4d12cdf2b453..d6320c63be45 100644
--- a/arch/x86/kernel/ldt.c
+++ b/arch/x86/kernel/ldt.c
@@ -34,7 +34,7 @@ static void flush_ldt(void *current_mm)
 }
 
 /* The caller must call finalize_ldt_struct on the result. LDT starts zeroed. */
-static struct ldt_struct *alloc_ldt_struct(int size)
+static struct ldt_struct *alloc_ldt_struct(unsigned int size)
 {
 	struct ldt_struct *new_ldt;
 	int alloc_size;

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

* Re: [patch] x86/ldt: make a size variable unsigned
  2016-12-08 10:56 [patch] x86/ldt: make a size variable unsigned Dan Carpenter
@ 2016-12-08 22:29 ` Andy Lutomirski
  2016-12-08 23:08 ` Thomas Gleixner
  2016-12-10  8:12 ` [tip:x86/cleanups] x86/ldt: Make a size argument unsigned tip-bot for Dan Carpenter
  2 siblings, 0 replies; 4+ messages in thread
From: Andy Lutomirski @ 2016-12-08 22:29 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, X86 ML,
	Dave Hansen, Andrew Morton, Andrey Ryabinin, Andy Lutomirski,
	linux-kernel, kernel-janitors

On Thu, Dec 8, 2016 at 2:56 AM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> My static checker complains that we put an upper bound on the "size"
> variable but not a lower bound.  The checker is not smart enough to know
> the possible ranges of "old_mm->context.ldt->size" from
> init_new_context_ldt() so it thinks maybe it could be negative.
>
> Let's make it unsigned to silence the warning and future proof the code
> a bit.

Acked-by: Andy Lutomirski <luto@kernel.org>

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

* Re: [patch] x86/ldt: make a size variable unsigned
  2016-12-08 10:56 [patch] x86/ldt: make a size variable unsigned Dan Carpenter
  2016-12-08 22:29 ` Andy Lutomirski
@ 2016-12-08 23:08 ` Thomas Gleixner
  2016-12-10  8:12 ` [tip:x86/cleanups] x86/ldt: Make a size argument unsigned tip-bot for Dan Carpenter
  2 siblings, 0 replies; 4+ messages in thread
From: Thomas Gleixner @ 2016-12-08 23:08 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Ingo Molnar, H. Peter Anvin, x86, Dave Hansen, Andrew Morton,
	Andrey Ryabinin, Andy Lutomirski, linux-kernel, kernel-janitors

On Thu, 8 Dec 2016, Dan Carpenter wrote:
> My static checker complains that we put an upper bound on the "size"
> variable but not a lower bound.  The checker is not smart enough to know
> the possible ranges of "old_mm->context.ldt->size" from
> init_new_context_ldt() so it thinks maybe it could be negative.
> 
> Let's make it unsigned to silence the warning and future proof the code
> a bit.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> diff --git a/arch/x86/kernel/ldt.c b/arch/x86/kernel/ldt.c
> index 4d12cdf2b453..d6320c63be45 100644
> --- a/arch/x86/kernel/ldt.c
> +++ b/arch/x86/kernel/ldt.c
> @@ -34,7 +34,7 @@ static void flush_ldt(void *current_mm)
>  }
>  
>  /* The caller must call finalize_ldt_struct on the result. LDT starts zeroed. */
> -static struct ldt_struct *alloc_ldt_struct(int size)
> +static struct ldt_struct *alloc_ldt_struct(unsigned int size)
>  {
>  	struct ldt_struct *new_ldt;
>  	int alloc_size;

So yes, this fixes the checker warning, but then it leaves the code in
inconsistent state:

	alloc_size = size * LDT_ENTRY_SIZE;

Why not doing the obvious

-	int alloc_size;
+	unsigned int alloc_size;

right away?

We have lots of places where we 'fixup' stuff and leave the rest untouched,
which then causes trouble a few month down the road. Probably not in this
case, but still.

I'm all for addressing such issues, but then please take the time aside of
mechanically fixing the checker fallout to make the overall usage
consistent.

Thanks,

	tglx

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

* [tip:x86/cleanups] x86/ldt: Make a size argument unsigned
  2016-12-08 10:56 [patch] x86/ldt: make a size variable unsigned Dan Carpenter
  2016-12-08 22:29 ` Andy Lutomirski
  2016-12-08 23:08 ` Thomas Gleixner
@ 2016-12-10  8:12 ` tip-bot for Dan Carpenter
  2 siblings, 0 replies; 4+ messages in thread
From: tip-bot for Dan Carpenter @ 2016-12-10  8:12 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: aryabinin, linux-kernel, tglx, dave.hansen, akpm, mingo, hpa,
	dan.carpenter, luto

Commit-ID:  296dc5806de57dc84fce000d60fc201ba40f96e8
Gitweb:     http://git.kernel.org/tip/296dc5806de57dc84fce000d60fc201ba40f96e8
Author:     Dan Carpenter <dan.carpenter@oracle.com>
AuthorDate: Thu, 8 Dec 2016 13:56:03 +0300
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Sat, 10 Dec 2016 00:24:39 +0100

x86/ldt: Make a size argument unsigned

My static checker complains that we put an upper bound on the "size"
argument but not a lower bound.  The checker is not smart enough to know
the possible ranges of "old_mm->context.ldt->size" from
init_new_context_ldt() so it thinks maybe it could be negative.

Let's make it unsigned to silence the warning and future proof the code
a bit.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Acked-by: Andy Lutomirski <luto@kernel.org>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: kernel-janitors@vger.kernel.org
Cc: Andrey Ryabinin <aryabinin@virtuozzo.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Link: http://lkml.kernel.org/r/20161208105602.GA11382@elgon.mountain
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---
 arch/x86/kernel/ldt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/ldt.c b/arch/x86/kernel/ldt.c
index 6707039..e25b668 100644
--- a/arch/x86/kernel/ldt.c
+++ b/arch/x86/kernel/ldt.c
@@ -34,7 +34,7 @@ static void flush_ldt(void *current_mm)
 }
 
 /* The caller must call finalize_ldt_struct on the result. LDT starts zeroed. */
-static struct ldt_struct *alloc_ldt_struct(int size)
+static struct ldt_struct *alloc_ldt_struct(unsigned int size)
 {
 	struct ldt_struct *new_ldt;
 	int alloc_size;

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

end of thread, other threads:[~2016-12-10  8:14 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-08 10:56 [patch] x86/ldt: make a size variable unsigned Dan Carpenter
2016-12-08 22:29 ` Andy Lutomirski
2016-12-08 23:08 ` Thomas Gleixner
2016-12-10  8:12 ` [tip:x86/cleanups] x86/ldt: Make a size argument unsigned tip-bot for Dan Carpenter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).