All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Stéphane Aulery" <saulery-lkSrsyIBln0dnm+yROfE0A@public.gmane.org>
To: wharms-fPG8STNUNVg@public.gmane.org,
	794947-61a8vm9lEZVf4u+23C9RwQ@public.gmane.org
Cc: linux-man-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: Bug#794947: manpages-dev: printf(3) example: possible integer overflow
Date: Thu, 18 Feb 2016 20:18:33 +0100	[thread overview]
Message-ID: <56C61909.20606@legtux.org> (raw)
In-Reply-To: <56C4E269.5020108-fPG8STNUNVg@public.gmane.org>

Hello Walter,

Le 17/02/2016 22:13, walter harms a écrit :
>>
>> Jakub Wilk reported a possible integer overflow in make_message example :
>>
>>> The example in the printf(3) manpages looks like this (with boring parts
>>> omitted):
>>>
>>> int n;
>>> /* ... */
>>>    n = vsnprintf(p, size, fmt, ap);
>>>     /* ... */
>>>     if (n < 0) {
>>>         /* ... */
>>>         return NULL;
>>>     }
>>>     /* ... */
>>>     size = n + 1;
>>>
>>>
>>> But vsnprintf could return INT_MAX, which would then cause "n + 1" to
>>> overflow.
>>>
>>> (AFAICS, the glibc vsnprintf implementation never returns INT_MAX, but
>>> it could in principle.)
>>>
>>> I'd suggest changing "n < 0" to "n < 0 || n == INT_MAX".
>>
>
> the bug is real, the type of size should be size_t (in my original post it was int)
> That would make the error check useless, so we would need to store
> the vsnprintf return value in an int.
>
> The problem is that the idea was to have a simple example and cluttering
> it with error checks will make it hard to read. How many people would
> notice that size_t is unsigned and n is signed ? (i added an comment).
>
> IMHO we should simply add a sentence that "examples are examples and
> will not check for every possible error condition."


I agree with the general idea: the examples must remain so. They must 
also be correct. Tough choice!

I will not put a note on this page about it, nor on the other, too much 
for so little.

man-pages.7 specifically requests:

      Example programs shoulds be fairly short (preferably less than 100 
lines;
      Ideally less than 50 lines).

      Example programs shoulds do error checking after-system calls and
      library function calls.

So I will do a patch with your new corrected version that is very readable.

Thanks a lot for your help.

Regards,

-- 
Stéphane Aulery
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

      parent reply	other threads:[~2016-02-18 19:18 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-17 12:40 Bug#794947: manpages-dev: printf(3) example: possible integer overflow Stéphane Aulery
     [not found] ` <e62670273dd84e658fe32cda6e16e94b-lkSrsyIBln0dnm+yROfE0A@public.gmane.org>
2016-02-17 21:13   ` walter harms
     [not found]     ` <56C4E269.5020108-fPG8STNUNVg@public.gmane.org>
2016-02-18 19:18       ` Stéphane Aulery [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=56C61909.20606@legtux.org \
    --to=saulery-lksrsyibln0dnm+yrofe0a@public.gmane.org \
    --cc=794947-61a8vm9lEZVf4u+23C9RwQ@public.gmane.org \
    --cc=linux-man-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=wharms-fPG8STNUNVg@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.