linux-kernel - Re: [PATCH] ufs: Add table lookup to set d_type based on mode & S

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: Thu, 13 Jun 2024 21:40:47 +0100
From: Al Viro <viro@...iv.linux.org.uk>
To: Abhinav Jain <jain.abhinav177@...il.com>
Cc: dushistov@...l.ru, linux-kernel@...r.kernel.org,
	skhan@...uxfoundation.org, javier.carrasco.cruz@...il.com
Subject: Re: [PATCH] ufs: Add table lookup to set d_type based on mode &
 S_IFMT

On Thu, Jun 13, 2024 at 08:26:50PM +0000, Abhinav Jain wrote:
> Add usf_de_type_mapping structure to map file mode masks to dir entries.
> Add a static ufs_type_table array to map file mode to dir entries.
> Remove switch and add a table based lookup for the mapping.
> Add ARRAY_SIZE macro on ufs_type_table to eliminate checkpatch warning.

For one thing, ARRAY_SIZE is already defined.  Finding the header
in question (and figuring out if its include needs to be added) is
left as an exercise for reader.

For another, you have added a copy of that array to *every* *file*
that inludes "util.h".  Finding the number of those files is, again,
left as an exercise for reader.

Finally, this

> +	for (i = 0; i < ARRAY_SIZE(ufs_type_table); i++) {
> +		if ((mode & S_IFMT) == ufs_type_table[i].mode_mask) {
> +			de->d_u.d_44.d_type = ufs_type_table[i].d_type;
> +			break;
> +		}

should've raised an arseload of mental red flags.  That loop is
bloody ridiculous, even if you don't bother to check what other
similar filesystems actually do in the counterpart of that logics.

"Table lookup" does *NOT* refer to that.  What you've got is strictly
worse than the original switch, and that takes some doing.

NAK.