linux-kernel - Re: [PATCH v2 02/10] fs/ntfs3: Add initialization of super block

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:   Fri, 21 Aug 2020 12:39:06 -0700
From:   Joe Perches <joe@...ches.com>
To:     Konstantin Komarov <almaz.alexandrovich@...agon-software.com>,
        "viro@...iv.linux.org.uk" <viro@...iv.linux.org.uk>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>
Cc:     Pali Rohár <pali@...nel.org>
Subject: Re: [PATCH v2 02/10] fs/ntfs3: Add initialization of super block

On Fri, 2020-08-21 at 16:25 +0000, Konstantin Komarov wrote:
> Initialization of super block for fs/ntfs3
[]
> diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
[]
> +
> +/**
> + * ntfs_trace() - print preformated ntfs specific messages.
> + */
> +void __ntfs_trace(const struct super_block *sb, const char *level,
> +		  const char *fmt, ...)

This is a printk mechanism.

I suggest renaming this __ntfs_trace function to ntfs_printk
as there is a naming expectation conflict with the tracing
subsystem.

> +{
> +	struct va_format vaf;
> +	va_list args;
> +
> +	va_start(args, fmt);
> +	vaf.fmt = fmt;
> +	vaf.va = &args;
> +	if (!sb)
> +		printk("%sntfs3: %pV", level, &vaf);
> +	else
> +		printk("%sntfs3: %s: %pV", level, sb->s_id, &vaf);
> +	va_end(args);
> +}

Also it would be rather smaller overall object code to
change the macros and uses to embed the KERN_<LEVEL> into
the format and remove the const char *level argument.

Use printk_get_level to retrieve the level from the format.

see fs/f2fs/super.c for an example.

This could be something like the below with a '\n' addition
to the format string to ensure that messages are properly
terminated and cannot be interleaved by other subsystems
content that might be in another simultaneously running
thread starting with KERN_CONT.

void ntfs_printk(const struct super_block *sb, const char *fmt, ...)
{
	struct va_format vaf;
	va_list args;
	int level;

	va_start(args, fmt);

	level = printk_get_level(fmt);
	vaf.fmt = printk_skip_level(fmt);
	vaf.va = &args;
	if (!sb)
		printk("%c%cntfs3: %pV\n",
		       KERN_SOH_ASCII, level, &vaf);
	else
		printk("%c%cntfs3: %s: %pV\n",
		       KERN_SOH_ASCII, level, sbi->sb->s_id, &vaf);

	va_end(args);
}

> +
> +/* prints info about inode using dentry case if */
> +void __ntfs_inode_trace(struct inode *inode, const char *level, const char *fmt,

ntfs_inode_printk

> +			...)
> +{
> +	struct super_block *sb = inode->i_sb;
> +	ntfs_sb_info *sbi = sb->s_fs_info;
> +	struct dentry *dentry;
> +	const char *name = "?";
> +	char buf[48];
> +	va_list args;
> +	struct va_format vaf;
> +
> +	if (!__ratelimit(&sbi->ratelimit))
> +		return;
> +
> +	dentry = d_find_alias(inode);
> +	if (dentry) {
> +		spin_lock(&dentry->d_lock);
> +		name = (const char *)dentry->d_name.name;
> +	} else {
> +		snprintf(buf, sizeof(buf), "r=%lx", inode->i_ino);
> +		name = buf;
> +	}
> +
> +	va_start(args, fmt);
> +	vaf.fmt = fmt;
> +	vaf.va = &args;
> +	printk("%s%s on %s: %pV", level, name, sb->s_id, &vaf);
> +	va_end(args);
> +
> +	if (dentry) {
> +		spin_unlock(&dentry->d_lock);
> +		dput(dentry);
> +	}
> +}

Remove level and use printk_get_level as above.
Format string should use '\n' termination here too.