linux-ext4 - Re: [PATCH] ext2: reduce redundant check of '*options'

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] [day] [month] [year] [list]

Message-ID: <20110118222419.GC5070@quack.suse.cz>
Date:	Tue, 18 Jan 2011 23:24:19 +0100
From:	Jan Kara <jack@...e.cz>
To:	Ted Ts'o <tytso@....edu>
Cc:	Jan Kara <jack@...e.cz>, Simon Xu <xu.simon@...cle.com>,
	linux-ext4@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ext2: reduce redundant check of '*options'

On Tue 18-01-11 08:43:45, Ted Ts'o wrote:
> On Mon, Jan 17, 2011 at 06:02:03PM +0100, Jan Kara wrote:
> > Seriously, if you want to do some useful fixing then have a look for
> > example at making ext3/4 (in fact JBD/JBD2) handle transaction allocation
> > failures gratefully.  Currently we just loop in start_this_handle()
> > infinitely until we can make the allocation which is a bit dumb. So it
> > would be good, to make as many places as possible able to handle ENOMEM
> > from start_this_handle() and propagate the error to user space. In some
> > cases, it might not be easily possible (e.g. during writeout of dirty
> > memory, where proper handling needs more thought) but lots of cases
> > should be rather simple and need just auditing the code paths. Thanks.
> 
> Uh, this is actually a rather subtle set of fixes that you are
> proposing, because it won't be obvious to someone who hasn't paid very
> careful attention to who calls certain functions (i.e., such as the
> writepages function), which get propagated to user space, and which
> will just cause mysterious data corruption of files --- especially
> since the writeback daemon doesn't check error returns (!!!).  So if
> someone changed writepages to return ENOMEM, and then said, "How
> delightful!  My work is done...", it would result in nothing getting
> back to userspace in most cases, and the writeback code would mark the
> pages as clean, and on the next reboot, the data would be lost.
  I agree that the patches will need a careful review because of catches
like you mention.

> So this is *not* a change that I would recommend to people who are
> just beginning to learn how to program the kernel, and have been
> taught by some kernel programmers that the best thing to do is to take
> trivial-style changes and submit them upstream.
 I agree, maybe I've shooted too high (feel free to suggest better topic),
but we'll see what the guy comes up with. Personally, I find it more useful
to correct bad patches trying to achieve good things than spending time
shooting down trivial code rearrangements...

								Honza
-- 
Jan Kara <jack@...e.cz>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html