linux-kernel - Re: [PATCH] seq_file: Fix seq_putc() to be consistent with seq

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: <20140929112538.3929a9a2@gandalf.local.home>
Date:	Mon, 29 Sep 2014 11:25:38 -0400
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Petr Mladek <pmladek@...e.cz>
Cc:	LKML <linux-kernel@...r.kernel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Al Viro <viro@...iv.linux.org.uk>,
	Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [PATCH] seq_file: Fix seq_putc() to be consistent with
 seq_puts()

On Mon, 29 Sep 2014 16:41:22 +0200
Petr Mladek <pmladek@...e.cz> wrote:


> This patch moves set_putc() from b) to c) because it leaves mess in the
> last byte.

Which removes the "b" section, and makes seq_putc() and seq_puts()
function the same.

> 
> 
> The semantic looks weird in all categories:
> 
> add a) They report error even when there was not a real overflow.
>     Well, they do not know if there was overflow because
>     bitmap_scnprintf() does not report if it shrunk
>     the data or not.
> 
> add b) It does not report error even when seq_overflow() might later
>     report overflow.
> 
> add c) Any overflow invalidates the whole buffer because there might
>     be mess at the end. The overflow is not checked by read functions,
>     so they allow to read the mess.
> 
> 
> I think that reasonable semantic would be:
> 
>   + either signalize overflow another way (extra struct member and
>     leave count to point to the valid data) or always fill valid
>     data until the end of the buffer.
> 
>     The second variant might be problem if we add only half of
>     an escape sequence.
> 
> 
>   + always return error when seq_overflow() would return overflow;
>     in fact, the full buffer means that the last write operation
>     was most likely incomplete

I think this is probably the best option.

> 
>     Also we might rename it from "overflow" to "full" or "filled"
>     or "sealed" or "complete" that might describe the state more
>     precisely because there is not a real overflow in some situations

We could make overflow be m->count > m->size, which would mean that
m->count == m->size (full) isn't a problem. Although, I would have to
audit the kernel to see if anything just assumes that m->count is
always less than m->size.

This would mean overflow is the correct term, and wouldn't have to
change it.

Anyway, that should be a second patch, and we should get my current
patch (the one to make putc and puts the same) in first, as that
actually fixes the inconsistency between the two.

I'll post another patch to try to make seq_file operations a bit more
consistent. Perhaps we can have m->count be what would have been
written.

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