On Fri 27-09-24 08:50:19, Theodore Ts'o wrote: > 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 Right, loops like these are not something we should be fixing in the kernel. > 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(). Agreed as well. I'll also note that keeping such flag uptodate is not as simple as it seems because there are various places that may be allocating blocks beyond EOF (for example extending writes) and that rely on ext4_truncate() removing them so one needs to be careful to capture all the places where the "truncated" state needs to be cleared. Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR
