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]

Date:	Mon, 01 Sep 2014 14:33:42 +0300
From:	Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To:	Geert Uytterhoeven <geert@...ux-m68k.org>
Cc:	Andy Shevchenko <andy.shevchenko@...il.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, 2014-09-01 at 12:59 +0200, Geert Uytterhoeven wrote:
> Hi Andy,
> 
> On Mon, Sep 1, 2014 at 12:15 PM, Andy Shevchenko
> <andy.shevchenko@...il.com> wrote:
> >>>> 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 don't think complaining about a too-small buffer prematurely hurts.
> 
> > I think the requirement for one groupsize is quite okay.
> 
> Then you will loose data if the buffer is too small.
> 
> >> 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".
> 
> The idea of seq_*() is that it will retry with a bigger bufsize if there's
> not enough space.

Fair enough.

So, summarize above, the check before hex_dump_to_buffer() should care
about maximum possible buffer needed for one line. But we have already
rowsize calculated (we actually can't rely on groupsize in this case).

Do you agree with formula rowsize * 3 + 2 + rowsize + 1?

-- 
Andy Shevchenko <andriy.shevchenko@...el.com>
Intel Finland Oy

--
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/