linux-kernel - Re: [PATCH] [PATCH] vfs/ext4: Fixed a potential problem related to an infinite loop

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: <20240927155019.GA365622@mit.edu>
Date: Fri, 27 Sep 2024 08:50:19 -0700
From: "Theodore Ts'o" <tytso@....edu>
To: Max Brener <linmaxi@...il.com>
Cc: adilger.kernel@...ger.ca, viro@...iv.linux.org.uk, brauner@...nel.org,
        jack@...e.cz, linux-ext4@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH] [PATCH] vfs/ext4: Fixed a potential problem related to
 an infinite loop

On Fri, Sep 27, 2024 at 01:11:03AM +0300, Max Brener wrote:
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=219306
> 
> This patch fixes a potential infinite journal-truncate-print
> problem.  When systemd's journald is called, ftruncate syscall is
> called. If anywhere down the call stack of ftruncate a printk of
> some sort happens, it triggers journald again therefore an infinite
> loop is established.

This isn't a good justification for this change; in general, whenever
you have code paths which get triggered when a logging daemon is
triggered, whether it's systemd-journald, or syslog, and this can
cause this kind of infinite loop.  For example, suppose you are using
remote logging (where a log message gets sent over the network via the
remote syslog facility), and anything in the networking stack triggers
a printk, that will also trigger an "infinite loop".  This falls in
the "Doctor, doctor, it hurts when I do that --- so don't do that!"

In this particular situation, journald is doing something silly/stupid
which is whenver a message is logged, it is issuing a no-op ftruncate
to the journald log file.  It's also worth noting that ext4's truncate
path does *not* trigger a printk unless something really haw gone
wrong (e.g., a WARN_ON when a kernel bug has happened and flags in the
in-memory get erronously set, or the file system gets corrupted and
this gets reported via ext4_error()).  The reporter discovered this by
explicitly adding a printk in their privatea kernel sources, and in
general, when you add random changes to the kernel, any unfortunate
consequences are not something that upstream code can be expected to
defend against.

For context, see: https://bugzilla.kernel.org/show_bug.cgi?id=219306

We can justify an optimization here so that in the case of
silly/stupid userspace programs which are constnatly calling
truncate(2) which are no-ops, we can optimize ext4's handling of these
silly/stupid programs.  The ext4_truncate() code path causes starting
a journal handle, adding the inode to the orphan list, and then
removing it at the end of the truncate.  In the case where sopme
program calls truncate() in a tight loop, we can optimize the
behaviour.  It's not a high priority optimization, but if given that
we can't necessarily change silly/stupid userspace programmers, it can
be something that we can do if the patch is too invasive.

HOWEVER....

> To fix this issue:
> Add  a new inode flag S_TRUNCATED which helps in stopping such an infinite loop by marking an in-memory inode as already truncated.

Adding a generic VFS-level flag is not something that we can justify
here.  The VFS maintainers would NACK such a change, and deservedly
so.

What I had in mind was to define a new EXT4 state flag, say,
EXT4_STATE_TRUNCATED, and then test, set, and clear it using
ext4_{test,set,clear}_inode_state().

Cheers,

					- Ted