| Message ID | 20170601202724.77597-2-luc.vanoostenryck@gmail.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On 01/06/17 21:27, Luc Van Oostenryck 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. > > However these functions take an unsigned byte count (size_t) > and so the value can't really be negative. My patch wasn't as careful as this. (and I was too lazy to check that the kernel functions took a size_t parameter). Assuming all functions take a size_t parameter, looks good. ATB, Ramsay Jones > > 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(-) > > 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; > } > -- 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
On Fri, Jun 2, 2017 at 2:16 AM, Ramsay Jones <ramsay@ramsayjones.plus.com> wrote: > > Assuming all functions take a size_t parameter, looks good. Even if they/some don't, it will be OK because it's how sparse interpret the constant that matter. -- 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
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; }
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(-)