linux-kernel - Re: [PATCH v4 2/3] hfs: correct superblock flags

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]

Message-ID: <a9296e54612d5f23c4e4acc671916f2f81a03e1f.camel@dubeyko.com>
Date: Tue, 22 Jul 2025 12:06:48 -0700
From: Viacheslav Dubeyko <slava@...eyko.com>
To: Yangtao Li <frank.li@...o.com>, glaubitz@...sik.fu-berlin.de
Cc: linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 2/3] hfs: correct superblock flags

On Tue, 2025-07-22 at 01:13 -0600, Yangtao Li wrote:
> We don't support atime updates of any kind,
> because hfs actually does not have atime.
> 
>    dirCrDat:      LongInt;    {date and time of creation}
>    dirMdDat:      LongInt;    {date and time of last modification}
>    dirBkDat:      LongInt;    {date and time of last backup}
> 
>    filCrDat:      LongInt;    {date and time of creation}
>    filMdDat:      LongInt;    {date and time of last modification}
>    filBkDat:      LongInt;    {date and time of last backup}
> 

I have troubles with the current state of the comment. If I am trying
to find dirCrDat, dirMdDat, ..., filBkDat in HFS driver source code,
then I cannot find it. So, I prefer to cite the HFS declaration here:

/* The catalog record for a file */
struct hfs_cat_file {
<skipped>
	__be32 CrDat;			/* The creation date */
	__be32 MdDat;			/* The modified date */
	__be32 BkDat;			/* The last backup date */
<skipped>
} __packed;

/* the catalog record for a directory */
struct hfs_cat_dir {
<skipped>
	__be32 CrDat;			/* The creation date */
	__be32 MdDat;			/* The modification date */
	__be32 BkDat;			/* The last backup date */
<skipped>
} __packed;

I assume that you showing information from HFS specification. So, it
makes sense to mention that you are using the HFS specification details
and, maybe, share more detailed explanation of this. Let's combine HFS
driver declarations and citation of HFS specification.

The rest looks pretty good to me.

Thanks,
Slava.

> Signed-off-by: Yangtao Li <frank.li@...o.com>
> ---
> v4:
> -add both SB_NODIRATIME and SB_NOATIME flags
>  fs/hfs/super.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/hfs/super.c b/fs/hfs/super.c
> index fe09c2093a93..417950d388b4 100644
> --- a/fs/hfs/super.c
> +++ b/fs/hfs/super.c
> @@ -331,7 +331,7 @@ static int hfs_fill_super(struct super_block *sb,
> struct fs_context *fc)
>  	sbi->sb = sb;
>  	sb->s_op = &hfs_super_operations;
>  	sb->s_xattr = hfs_xattr_handlers;
> -	sb->s_flags |= SB_NODIRATIME;
> +	sb->s_flags |= SB_NODIRATIME | SB_NOATIME;
>  	mutex_init(&sbi->bitmap_lock);
>  
>  	res = hfs_mdb_get(sb);