linux-kernel - Re: [PATCH take 3][RFC] fat: Save FAT root directory timestamps to volume label

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <878wgcsd1p.fsf@devron.myhome.or.jp>
Date:	Fri, 18 Sep 2009 20:33:06 +0900
From:	OGAWA Hirofumi <hirofumi@...l.parknet.co.jp>
To:	Jorg Schummer <ext-jorg.2.schummer@...ia.com>
Cc:	linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
	jorg@....net
Subject: Re: [PATCH take 3][RFC] fat: Save FAT root directory timestamps to volume label

Jorg Schummer <ext-jorg.2.schummer@...ia.com> writes:

> Standard FAT implementations cannot store any of the FAT root directory's
> timestamps. This commit adds the mount option 'rootts', which allows saving
> the FAT root directory timestamps as the timestamps of the FAT volume label
> directory entry. At least Mac OS X is known to support the same mechanism
> and interoperate with this commit.
>
> When mounting, the following values can be specified for the 'rootts' mount
> option:
>
>   "rootts=ignore"   ignores root directory timestamps. All timestamps are
>                     reset to 0 (1/1/1970). This has been the FAT behaviour
>                     prior to this patch.
>
>   "rootts=preserve" tries to load and save the root directory's timestamps
>                     if a volume label entry exists. The mtime and atime are
>                     corrected based on root directory entries' ctime. This
>                     is the default.
>
>   "rootts=save"     tries to load and save the root directory's timestamps.
>                     The mtime and atime are corrected based on root
>                     directory entries' ctime. If the root directory was
>                     accessed but no volume label entry exists, the label
>                     "NO NAME" is created.

Um..., I'm still not sure though. rootts=save may not be needed for
this, because it seems a bit complex, and it can do on userland...

And some quick review of implement..

> +	if (inode->i_ino == MSDOS_ROOT_INO) {
> +		if (sbi->options.root_ts == FAT_ROOT_TS_IGNORE)
> +			return 0;
> +
> +		/* Write the timestamps of a FAT file system's root directory
> +		 * as the timestamps of the file system's label dir entry. */
> +
> +		if (!MSDOS_I(inode)->i_pos) {
> +			struct buffer_head *bh;
> +			struct msdos_dir_entry *de;
> +			loff_t i_pos;
> +
> +			if (sbi->options.root_ts != FAT_ROOT_TS_SAVE)
> +				/* No label present, but we should not
> +				   create one. Thus not complaining. */
> +				return 0;
> +
> +			printk(KERN_INFO "FAT: creating volume label "
> +			       "on %s to save root dir timestamps\n",
> +			       sb->s_id);
> +
> +			err = fat_add_label_entry(inode, NULL);
> +			if (err)
> +				return err;
> +			err = fat_get_label_entry(inode, &bh, &de, &i_pos);
> +			if (err)
> +				return err;

In this path, we don't have inode->i_mutex, so to look directry up is
wrong. I think adding label may move to userland.

I guess we should do all of preparation in fill_super().

> +			remove_inode_hash(inode);
> +			fat_attach(inode, i_pos);
> +			insert_inode_hash(inode);
> +			brelse(bh);

Root inode should never change the position, so this shouldn't be
needed.

Thanks.
-- 
OGAWA Hirofumi <hirofumi@...l.parknet.co.jp>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/