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: Windows password security audit tool. GUI, reports in PDF.

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

Message-Id: <1253276050.9376.1335476075@webmail.messagingengine.com>
Date:	Fri, 18 Sep 2009 15:14:10 +0300
From:	jorg@....net
To:	"OGAWA Hirofumi" <hirofumi@...l.parknet.co.jp>,
	"Jorg Schummer" <ext-jorg.2.schummer@...ia.com>
Cc:	linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH take 3][RFC] fat: Save FAT root directory timestamps to
 volume label

Hello and thanks for your comments!

On Fri, 18 Sep 2009 20:33 +0900, "OGAWA Hirofumi"
<hirofumi@...l.parknet.co.jp> wrote:
> 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=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...

It's true, that is quite of a hack after all. I just thought I'll try to
add the non-plus-ultra-feature on top of what Mac OS can do already.

But you're right, I'll get rid of the label creation. (And I think I
should get rid of the time stamp correction as well. If there is no
timestamp to be loaded, then 1970/1/1 should suffice. In case anyone
thinks that the timestamp 'correction' is anyhow useful, let me know...)

> 
> And some quick review of implement..
> 
> > +	if (inode->i_ino == MSDOS_ROOT_INO) {
> 
> 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().

True. This code is only needed for the time stamp creation, so it'll go
in any case.

> 
> > +			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.

Like above, this will be removed.

If 'preserve' (i.e. saving and loading the timestamp only IF a label is
present) is the default, I'll remove the 'rootts=preserve' option.
However, should there still be an option to switch this feature off
completely ('rootts=ignore')?

Thanks very much for your comments, the next version will come soon!

Jörg
--
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/