linux-kernel - Re: [PATCH v3 1/5] seq_file: provide an analogue of print_hex

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHp75VeNegxwTB617mRXBomERjLbATASMM0tReUgZWprQEnXng@mail.gmail.com>
Date:	Mon, 1 Sep 2014 13:15:59 +0300
From:	Andy Shevchenko <andy.shevchenko@...il.com>
To:	Geert Uytterhoeven <geert@...ux-m68k.org>
Cc:	Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
	Al Viro <viro@...iv.linux.org.uk>,
	Tadeusz Struk <tadeusz.struk@...el.com>,
	Herbert Xu <herbert@...dor.apana.org.au>,
	Mauro Carvalho Chehab <m.chehab@...sung.com>,
	Helge Deller <deller@....de>,
	Ingo Tuchscherer <ingo.tuchscherer@...ibm.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Joe Perches <joe@...ches.com>, Marek Vasut <marex@...x.de>
Subject: Re: [PATCH v3 1/5] seq_file: provide an analogue of print_hex_dump()

On Mon, Sep 1, 2014 at 12:25 PM, Geert Uytterhoeven
<geert@...ux-m68k.org> wrote:
> Hi Andy,
>
> On Mon, Sep 1, 2014 at 11:09 AM, Andy Shevchenko
> <andriy.shevchenko@...ux.intel.com> wrote:
>> On Mon, 2014-09-01 at 10:58 +0200, Geert Uytterhoeven wrote:
>>> On Mon, Sep 1, 2014 at 10:36 AM, Andy Shevchenko
>>> <andriy.shevchenko@...ux.intel.com> wrote:
>>> >> ... and extra copying for no good reason.  Why not check that we have
>>> >> enough space in buffer and generate directly into it?  See what e.g.
>>> >> seq_escape() is doing...
>>> >
>>> > What about this variant?
>>>
>>> I think it needs a call to seq_set_overflow() in case the buffer is too small,
>>> so the caller will retry with a bigger buffer.
>>
>> Yes, in two places it would be useful to do.
>
> Two places? I see only one, just before calling hex_dump_to_buffer.

seq_putc doesn't set it as I can see.

>> But what the condition for "buffer is too small", the same groupsize * 2
>> + 1 or you mean something else?
>
> "groupsize * 2 + 1" is not the amount of bytes hex_dump_to_buffer() wants
> to write. It's only the size for one word.
>
> You could check if there are at least "32 * 3 + 2 + 32 + 1" bytes (your
> old linebuf size) available.

This is a good question why this number? What if we have to print only
one byte (as different groupsize)?
I think the requirement for one groupsize is quite okay.

> However, to protect against overflows if hex_dump_to_buffer() ever changes,
> I think it would be better to let hex_dump_to_buffer() indicate if the
> passed buffer was to small (it already checks the passed linebuflen).
> Then you can just check for that.

I thought about that. We may introduce either new call and make
current one the user of it or change all occurrences.
Nevertheless, currently it will print only one groupsize if there is
enough room for it but for two or more.
Thus, I prefer to keep the behaviour "print until we can".

-- 
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/