diff mbox

[v2,1/3] memcpy()'s byte count is unsigned

Message ID 20170603074727.66945-2-luc.vanoostenryck@gmail.com (mailing list archive)
State Mainlined, archived
Headers show

Commit Message

Luc Van Oostenryck June 3, 2017, 7:47 a.m. UTC
The checker part of sparse does some checking on memcpy(),
memset(), copy_{from,to}_user() byte count and warn if the
value is known to be too large. The comparison is done with
signed numbers and it also warns if the value is negative.

However these functions take an unsigned byte count (size_t)
and so the value can't really be negative.

Additionaly, the number of bits used by sparse internally may not
be the same as the one used for the target's size_t. So sparse's
check against negative value may not be the same as checking if
the target's value would be so-large-than-the-upper-bit-is-set.

Change this by removing the test for negative values and simply
do an unsigned compare.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 sparse.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Christopher Li June 5, 2017, 8:52 p.m. UTC | #1
On Sat, Jun 3, 2017 at 12:47 AM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> The checker part of sparse does some checking on memcpy(),
> memset(), copy_{from,to}_user() byte count and warn if the
> value is known to be too large. The comparison is done with
> signed numbers and it also warns if the value is negative.


I see you have the V1 in git tree in the 0/3 email.

Do you have the V2 in the git tree some where as well?
I can't seem to find the V2 version of the 0/3 email.

Chris
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Luc Van Oostenryck June 5, 2017, 10:16 p.m. UTC | #2
On Mon, Jun 5, 2017 at 10:52 PM, Christopher Li <sparse@chrisli.org> wrote:
> On Sat, Jun 3, 2017 at 12:47 AM, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
>> The checker part of sparse does some checking on memcpy(),
>> memset(), copy_{from,to}_user() byte count and warn if the
>> value is known to be too large. The comparison is done with
>> signed numbers and it also warns if the value is negative.
>
>
> I see you have the V1 in git tree in the 0/3 email.
>
> Do you have the V2 in the git tree some where as well?
> I can't seem to find the V2 version of the 0/3 email.

Ah, sorry, I've cut-&-pasted the cover letter between V1 & V2
and thus removed the 'v2' from the title.

The mail corresponding to V2's cover letter is:
    http://marc.info/?l=linux-sparse&m=149647605600827&w=4
but I'll resent it as a reply to this mail.

-- Luc
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Luc Van Oostenryck June 5, 2017, 10:20 p.m. UTC | #3
sparse will warn if memcpy() (and some others memcpy-like
functions) is called with a very large static byte count.
But this warning cannot be disabled and the limit is arbitrary
fixed at 100000.

The goal of this series is to allow to disable this warning if
found too bothersome or to allow to configure its limit.


Changes since v1:
- take in account Ramsay's remarks and suggestion:
  - fix some name mixups in the man page & commit message
  - use a limit of 0 as being equivalent to an infinite
    limit, effectively disabling the warning.
- somewhat rewrote the man page for -fmemcpy-max-count
- extend the limit's range

The series can also be found on the tree:
	git://github.com/lucvoo/sparse.git memcpy-max-count-v2

Luc Van Oostenryck (3):
  memcpy()'s byte count is unsigned
  add support for -Wmemcpy-max-count
  add support for -fmemcpy-max-count

 lib.c    | 20 ++++++++++++++++++++
 lib.h    |  2 ++
 sparse.1 | 17 +++++++++++++++++
 sparse.c |  6 +++---
 4 files changed, 42 insertions(+), 3 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Christopher Li June 6, 2017, 1:26 a.m. UTC | #4
On Mon, Jun 5, 2017 at 3:16 PM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
>
> Ah, sorry, I've cut-&-pasted the cover letter between V1 & V2
> and thus removed the 'v2' from the title.

Ah, I see. I actually read that email before but I forget about it.
I will take a look at the V2 git branch.

Chris
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/sparse.c b/sparse.c
index 02ab97743..1cb90e20d 100644
--- a/sparse.c
+++ b/sparse.c
@@ -152,9 +152,9 @@  static void check_byte_count(struct instruction *insn, pseudo_t count)
 	if (!count)
 		return;
 	if (count->type == PSEUDO_VAL) {
-		long long val = count->value;
-		if (val <= 0 || val > 100000)
-			warning(insn->pos, "%s with byte count of %lld",
+		unsigned long long val = count->value;
+		if (val > 100000ULL)
+			warning(insn->pos, "%s with byte count of %llu",
 				show_ident(insn->func->sym->ident), val);
 		return;
 	}