> Anyone else have an opinion?
tl;dr: seq_printf() whould return void.
Well, certainly *if* seq_printf returns a value, it should be consistent
with printf, i.e. length or -errno.
If it's going to be anything else, then it should be incompatible with
an integer, so attempted uses cause compile-
On Sat, 2013-09-14 at 05:53 +0100, Al Viro wrote:
> The bottom line: most of these guys could as well return void; we have
> few overflow checks and those could be made explicit. As it is,
> "return -1 on overflow" had been a mistake.
What do you think of adding last_ret and last_len to
struct se
On Sat, Sep 14, 2013 at 04:48:02AM +0100, Al Viro wrote:
> Overall: I suspect that Joe might be right. The very few callers that
> use the return value and use it correctly can bloody well call
> seq_overflow(), preferably with a detailed comment about the reasons
> for doing so. Anything that r
On Sat, Sep 14, 2013 at 04:05:21AM +0100, Al Viro wrote:
> Again, the normal return value of ->show() is 0 and that includes the case of
> overflow. THE ONLY reason to check for overflow early is when subsequent
> output of ->show() takes long to generate and we want to skip that and
> have seq_re
On Sat, Sep 14, 2013 at 11:49:51AM +0900, Tetsuo Handa wrote:
> Even bad code which has never tested failure case, the authors should already
> know that "seq_printf() returns 0 on success case".
It is designed so that not testing failure case is normal approach for the
majority of users.
> -
Kees Cook wrote:
> 3- some callers of seq_printf (incorrectly) use the return value as a
> length indication
Are there really?
Is somebody using the return value from seq_printf() like
pos = snprintf(buf, sizeof(buf) - 1, "%s", foo);
snprintf(buf + pos, sizeof(buf) - 1 - pos, "%s", bar);
?
On Fri, Sep 13, 2013 at 04:03:25PM -0700, Kees Cook wrote:
> Maybe I missed this somewhere in the thread, but I'm not sure I
> understand the move to "void". Here's what I see, please correct me:
>
> 1- seq_printf currently returns success/failure
> 2- some callers of seq_printf (correctly) use t
On Fri, 2013-09-13 at 16:03 -0700, Kees Cook wrote:
> On Fri, Sep 13, 2013 at 3:27 PM, Joe Perches wrote:
> > On Fri, 2013-09-13 at 15:53 -0400, George Spelvin wrote:
> >> > Maybe WARN_ONCE so it's easier to emit the format too.
> >>
> >> Good idea. And, if it's not too much trouble, a comment ex
On Fri, Sep 13, 2013 at 3:27 PM, Joe Perches wrote:
> On Fri, 2013-09-13 at 15:53 -0400, George Spelvin wrote:
>> > Maybe WARN_ONCE so it's easier to emit the format too.
>>
>> Good idea. And, if it's not too much trouble, a comment explaining
>> why it's deliberately omitted so the issue doesn't
> Maybe WARN_ONCE so it's easier to emit the format too.
Good idea. And, if it's not too much trouble, a comment explaining
why it's deliberately omitted so the issue doesn't arise again.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord
> Why would you want to artificially make the function diverge
> from the spec?
Because %n make it easy to convert a not-uncommon format string bug into
a code injection. Thus, poses a significant security vulnerability.
Since it's an obscure and rarely-used feature, it is straightforward
to el
On Thu, Sep 12, 2013 at 08:03:29AM +0100, Jan Beulich wrote:
> >>> On 11.09.13 at 22:18, Kees Cook wrote:
> > On Wed, Sep 11, 2013 at 1:06 PM, Joe Perches wrote:
> >> On Wed, 2013-09-11 at 12:30 -0700, Kees Cook wrote:
> >>> The %n format is not ignored, so remove the incorrect comment about it.
>>> On 12.09.13 at 09:31, Kees Cook wrote:
> On Thu, Sep 12, 2013 at 12:03 AM, Jan Beulich wrote:
> On 11.09.13 at 22:18, Kees Cook wrote:
>>> On Wed, Sep 11, 2013 at 1:06 PM, Joe Perches wrote:
On Wed, 2013-09-11 at 12:30 -0700, Kees Cook wrote:
> The %n format is not ignored, so
On Thu, Sep 12, 2013 at 12:03 AM, Jan Beulich wrote:
On 11.09.13 at 22:18, Kees Cook wrote:
>> On Wed, Sep 11, 2013 at 1:06 PM, Joe Perches wrote:
>>> On Wed, 2013-09-11 at 12:30 -0700, Kees Cook wrote:
The %n format is not ignored, so remove the incorrect comment about it.
>>>
>>> I t
>>> On 11.09.13 at 22:18, Kees Cook wrote:
> On Wed, Sep 11, 2013 at 1:06 PM, Joe Perches wrote:
>> On Wed, 2013-09-11 at 12:30 -0700, Kees Cook wrote:
>>> The %n format is not ignored, so remove the incorrect comment about it.
>>
>> I think it may be better to reimplement the ignoring.
>
> Yeah
Yeah, just found those too.
---
fs/proc/consoles.c | 2 +-
fs/proc/nommu.c | 20 ++--
fs/proc/task_mmu.c | 18 +-
fs/proc/task_nommu.c | 20 ++--
4 files changed, 30 insertions(+), 30 deletions(-)
diff --git a/fs/proc/consoles.c b/fs
On Wed, 2013-09-11 at 13:18 -0700, Kees Cook wrote:
> I think Dan's idea of a WARN_ON_ONCE() here is a good idea to help
> anyone accidentally trying to use it.
Maybe WARN_ONCE so it's easier to emit the format too.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
th
On Wed, Sep 11, 2013 at 1:06 PM, Joe Perches wrote:
> On Wed, 2013-09-11 at 12:30 -0700, Kees Cook wrote:
>> The %n format is not ignored, so remove the incorrect comment about it.
>
> I think it may be better to reimplement the ignoring.
Yeah, just had a quick look, and scanf doesn't use this co
On Wed, 2013-09-11 at 13:18 -0700, Kees Cook wrote:
> On Wed, Sep 11, 2013 at 1:06 PM, Joe Perches wrote:
> > On Wed, 2013-09-11 at 12:30 -0700, Kees Cook wrote:
> >> The %n format is not ignored, so remove the incorrect comment about it.
> >
> > I think it may be better to reimplement the ignorin
On Wed, 2013-09-11 at 12:30 -0700, Kees Cook wrote:
> The %n format is not ignored, so remove the incorrect comment about it.
I think it may be better to reimplement the ignoring.
Maybe:
---
lib/vsprintf.c | 18 +---
net/ipv4/fib_trie.c | 30 +--
net/ipv4/ping.c
20 matches
Mail list logo
