Re: 2.6.23-mm1: BUG in reiserfs_delete_xattrs
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Christoph Hellwig wrote: On Mon, Oct 15, 2007 at 02:31:03PM -0400, Jeff Mahoney wrote: Here's a patch I worked up the other night that kills off struct file completely from the xattr code. I've tested it locally. Looks like a merge of Dave's and my patch :) ACK from me, I don't care whether it's one or two patches. Yeah, it probably is. I did it from scratch since it was my mess, and the patches I saw were against -mm. *shrug* Likewise, I don't care if it's one or two. - -Jeff - -- Jeff Mahoney SUSE Labs -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.4-svn0 (GNU/Linux) Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org iD8DBQFHFiJHLPWxlyuTD7IRAojqAJwKS+eL1yCtUVHzBSFUxjjkW6KgPwCcDRUE Q1V7tCPcT9h0a8ahVmYn+ms= =5kMt -END PGP SIGNATURE- - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 2.6.23-mm1: BUG in reiserfs_delete_xattrs
Christoph Hellwig wrote:
On Mon, Oct 15, 2007 at 12:34:58AM +0200, Laurent Riffard wrote:
reiserfs_delete_xattrs
reiserfs_delete_inode
generic_delete_inode
generic_drop_inode
iput
do_unlinkat
sys_unlink
sys_enter_past_esp
I reported a similar BUG in 2.6.22-rc8-mm2 (see
http://lkml.org/lkml/2007/9/27/235). Dave Hansen sent a patch for it, I
tested it and it was OK for 2.6.22-rc8-mm2.
I tried this patch on 2.6.23-mm1, and it fixed the BUGs here too.
The delete path is a similar case as the one Dave fixed, also cause by
a NULL vfsmount passed to dentry_open, but through a different code-path.
Untested fix for this problem below:
Here's a patch I worked up the other night that kills off struct file
completely from the xattr code. I've tested it locally.
After several posts and bug reports regarding interaction with the NULL
nameidata, here's a patch to clean up the mess with struct file in the
reiserfs xattr code.
As observed in several of the posts, there's really no need for struct file
to exist in the xattr code. It was really only passed around due to the
f_op-readdir() and a_ops-{prepare,commit}_write prototypes requiring it.
reiserfs_prepare_write() and reiserfs_commit_write() don't actually use
the struct file passed to it, and the xattr code uses a private version of
reiserfs_readdir() to enumerate the xattr directories.
I do have patches in my queue to convert the xattrs to use reiserfs_readdir(),
but I guess I'll just have to rework those.
This is pretty close to the patch by Dave Hansen for -mm, but I didn't
notice it until after I wrote this up.
Signed-off-by: Jeff Mahoney [EMAIL PROTECTED]
---
fs/reiserfs/xattr.c | 111 ++--
1 file changed, 31 insertions(+), 80 deletions(-)
--- a/fs/reiserfs/xattr.c 2007-08-27 14:03:39.0 -0400
+++ b/fs/reiserfs/xattr.c 2007-10-14 22:11:05.0 -0400
@@ -191,28 +191,11 @@ static struct dentry *get_xa_file_dentry
dput(xadir);
if (err)
xafile = ERR_PTR(err);
- return xafile;
-}
-
-/* Opens a file pointer to the attribute associated with inode */
-static struct file *open_xa_file(const struct inode *inode, const char *name,
-int flags)
-{
- struct dentry *xafile;
- struct file *fp;
-
- xafile = get_xa_file_dentry(inode, name, flags);
- if (IS_ERR(xafile))
- return ERR_PTR(PTR_ERR(xafile));
else if (!xafile-d_inode) {
dput(xafile);
- return ERR_PTR(-ENODATA);
+ xafile = ERR_PTR(-ENODATA);
}
-
- fp = dentry_open(xafile, NULL, O_RDWR);
- /* dentry_open dputs the dentry if it fails */
-
- return fp;
+ return xafile;
}
/*
@@ -228,9 +211,8 @@ static struct file *open_xa_file(const s
* we're called with i_mutex held, so there are no worries about the directory
* changing underneath us.
*/
-static int __xattr_readdir(struct file *filp, void *dirent, filldir_t filldir)
+static int __xattr_readdir(struct inode *inode, void *dirent, filldir_t
filldir)
{
- struct inode *inode = filp-f_path.dentry-d_inode;
struct cpu_key pos_key; /* key of current position in the directory
(key of directory entry) */
INITIALIZE_PATH(path_to_entry);
struct buffer_head *bh;
@@ -374,23 +356,16 @@ static int __xattr_readdir(struct file *
*
*/
static
-int xattr_readdir(struct file *file, filldir_t filler, void *buf)
+int xattr_readdir(struct inode *inode, filldir_t filler, void *buf)
{
- struct inode *inode = file-f_path.dentry-d_inode;
- int res = -ENOTDIR;
- if (!file-f_op || !file-f_op-readdir)
- goto out;
+ int res = -ENOENT;
mutex_lock_nested(inode-i_mutex, I_MUTEX_XATTR);
-//down(inode-i_zombie);
- res = -ENOENT;
if (!IS_DEADDIR(inode)) {
lock_kernel();
- res = __xattr_readdir(file, buf, filler);
+ res = __xattr_readdir(inode, buf, filler);
unlock_kernel();
}
-//up(inode-i_zombie);
mutex_unlock(inode-i_mutex);
- out:
return res;
}
@@ -436,7 +411,7 @@ reiserfs_xattr_set(struct inode *inode,
size_t buffer_size, int flags)
{
int err = 0;
- struct file *fp;
+ struct dentry *dentry;
struct page *page;
char *data;
struct address_space *mapping;
@@ -454,18 +429,18 @@ reiserfs_xattr_set(struct inode *inode,
xahash = xattr_hash(buffer, buffer_size);
open_file:
- fp = open_xa_file(inode, name, flags);
- if (IS_ERR(fp)) {
- err = PTR_ERR(fp);
+ dentry = get_xa_file_dentry(inode, name, flags);
+ if (IS_ERR(dentry)) {
+ err = PTR_ERR(dentry);
goto out;
}
- xinode = fp-f_path.dentry-d_inode;
+ xinode = dentry
Re: 2.6.23-mm1: BUG in reiserfs_delete_xattrs
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Laurent Riffard wrote: Le 15.10.2007 20:31, Jeff Mahoney a écrit : Christoph Hellwig wrote: On Mon, Oct 15, 2007 at 12:34:58AM +0200, Laurent Riffard wrote: reiserfs_delete_xattrs reiserfs_delete_inode generic_delete_inode generic_drop_inode iput do_unlinkat sys_unlink sys_enter_past_esp I reported a similar BUG in 2.6.22-rc8-mm2 (see http://lkml.org/lkml/2007/9/27/235). Dave Hansen sent a patch for it, I tested it and it was OK for 2.6.22-rc8-mm2. I tried this patch on 2.6.23-mm1, and it fixed the BUGs here too. The delete path is a similar case as the one Dave fixed, also cause by a NULL vfsmount passed to dentry_open, but through a different code-path. Untested fix for this problem below: Here's a patch I worked up the other night that kills off struct file completely from the xattr code. I've tested it locally. Sorry Jeff, your patch does not apply on 2.6.23-mm1. The 'struct file' removal from reiserfs_xattr_ function is already in -mm (make-reiserfs-stop-using-struct-file-for-internal.patch). The Dave's patch I was refering to is this one: I'd guess not. This patch was actually against mainline. I should've specified. I can work up one against -mm later today if it's needed. - -Jeff - -- Jeff Mahoney SUSE Labs -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.4-svn0 (GNU/Linux) Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org iD8DBQFHE8wyLPWxlyuTD7IRAiJrAJ4nC6gwH1cFjWx6BI04O5fDIRftmACcD2wb whyXThHlIBK2phnZ6Pf8Pb8= =Kx6k -END PGP SIGNATURE- - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] add FIEMAP ioctl to efficiently map file allocation
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Andreas Dilger wrote: On Apr 12, 2007 12:22 +0100, Anton Altaparmakov wrote: This would say that the offset on disk can move at any time or that the data is compressed or encrypted on disk thus the data is not useful for direct disk access. This makes sense. Even for Reiserfs the same is true with packed tails, and I believe if FIBMAP is called on a tail it will migrate the tail into a block because this is might be a sign that the file is a kernel that LILO wants to boot. Actually, reiserfs_aop_bmap() returns 0 when the requested block is in a tail. There's a separate ioctl for unpacking them. - -Jeff - -- Jeff Mahoney SUSE Labs -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.5 (GNU/Linux) Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org iD8DBQFGH5l+LPWxlyuTD7IRAn5/AJ9VcocIcDGr9wtAlgGZuOAQWqVASwCfVdWM uLZQq1mkf8hsGXOpZtKQH5w= =AxnN -END PGP SIGNATURE- - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] ext3: htree entry integrity checking
This patch adds integrity checking to two htree paths that are missing it.
Currently, if a corrupted directory entry with rec_len=0 is encountered,
we still trust that the data is valid. This can cause an infinite loop
in htree_dirblock_to_tree() since the iteration loop will never make any
progress.
I initially had written a ext3_check_htree_entry(), but saw that
ext3_dir_entry_2 is used for both htree and regular directory entries. Since
ext3_check_dir_entry() is used for checking ext3_dir_entry_2, I used that.
Can someone confirm that this is correct? There are other places where
de-rec_len == 0 is used by itself and I'd be fine doing that too, but I
figure more integrity checking isn't a bad thing.
This fixes the problem described at:
http://projects.info-pull.com/mokb/MOKB-10-11-2006.html
Signed-off-by: Jeff Mahoney [EMAIL PROTECTED]
diff -ruNpX ../dontdiff linux-2.6.18.orig/fs/ext3/namei.c
linux-2.6.18.orig.devel/fs/ext3/namei.c
--- linux-2.6.18.orig/fs/ext3/namei.c 2006-11-09 00:06:37.0 -0500
+++ linux-2.6.18.orig.devel/fs/ext3/namei.c 2006-11-12 20:15:19.0
-0500
@@ -551,6 +551,11 @@ static int htree_dirblock_to_tree(struct
dir-i_sb-s_blocksize -
EXT3_DIR_REC_LEN(0));
for (; de top; de = ext3_next_entry(de)) {
+ if (!ext3_check_dir_entry(__FUNCTION__, dir, de, bh,
+ (char *)de - bh-b_data)) {
+ brelse(bh);
+ return ERR_BAD_DX_DIR;
+ }
ext3fs_dirhash(de-name, de-name_len, hinfo);
if ((hinfo-hash start_hash) ||
((hinfo-hash == start_hash)
@@ -617,6 +622,11 @@ int ext3_htree_fill_tree(struct file *di
if (start_hash 2 || (start_hash ==2 start_minor_hash==0)) {
de = (struct ext3_dir_entry_2 *) frames[0].bh-b_data;
de = ext3_next_entry(de);
+ if (!ext3_check_dir_entry(__FUNCTION__, dir, de, frames[0].bh,
+ (char *)de - frames[0].bh-b_data)) {
+ err = ERR_BAD_DX_DIR;
+ goto errout;
+ }
if ((err = ext3_htree_store_dirent(dir_file, 2, 0, de)) != 0)
goto errout;
count++;
--
Jeff Mahoney
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ext3: htree entry integrity checking
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Andreas Dilger wrote: On Nov 16, 2006 11:50 -0500, Jeff Mahoney wrote: Currently, if a corrupted directory entry with rec_len=0 is encountered, we still trust that the data is valid. This can cause an infinite loop in htree_dirblock_to_tree() since the iteration loop will never make any progress. Actually, I think Eric Sandeen was working on similar fixes already, and instead of doing a per-item check each time we look at the entry it does a full-block check the first time it is read (as ext2 does). This fixes the problem described at: http://projects.info-pull.com/mokb/MOKB-10-11-2006.html Would also be good to CC linux-ext4, where the ext3 maintainers live. Ok, thanks. If that's already in -mm, I'll use that one. - -Jeff - -- Jeff Mahoney SUSE Labs -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.2 (GNU/Linux) Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org iD8DBQFFXQIwLPWxlyuTD7IRApH7AJ9+/SFmd9bf8E741wvxw/6vdrUrdwCeJNEG eHZMo5RWUrLW5iDEqehjRlI= =lGRM -END PGP SIGNATURE- - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC][PATCH] Generic fallback for security xattrs
Christoph Hellwig wrote:
Very nice, and gets rid of lots of crap. Now that we started parsing
the attribute name in generic code we should deprecate the old
-{get,set,list,remove}xattr inode operations and make the helpers
James added a while ago mandatory for the future.
I'd be fine with making the generic xattr routines the default with a
few exceptions:
1) Pass the entire name of the xattr (including prefix) to the
underlying filesystem. ReiserFS needs the entire name, including the
prefix, to look up an xattr. Reconstructing the name again, when it was
available in the caller, is wasteful. Once the determination of the
prefix is made, the filesystem code can just skip the prefix length
anyway if it doesn't need it.
2) Keep i_op-listxattr. For xattrs like system.posix_acl_access, sure,
it makes sense. But when selinux or user xattrs are introduced, the name
and number of such xattrs will be unknown to a generic routine.
I have a series of patches for reiserfs xattrs that I'm still working
the kinks out of, mainly performance-wise. With the patch set applied,
reiserfs_{set,get,remove}xattr look identical to the generic versions,
except that find_xattr_handler_prefix doesn't modify the name.
-Jeff
--
Jeff Mahoney
SuSE Labs
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Access content of file via inodes
Kathy KN wrote: Good day all, How do I access/read the content of the files via using inodes or blocks that belong to the inode, at sys_link and vfs_link layer? I used bmap to access the blocks that belongs to the inodes, but getting access to the buffer_head's b_data doesn't seem to help. Hi Kathy - What you're trying to do is possible, but you need to go about it in a different way. Ignore the buffer cache completely and use the page cache; it's more appropriate for file contents. You have two options: If performance isn't critical, a simple approach would be to use your old_dentry pointer to dentry_open a file and then vfs_read from it to a buffer you allocate. Make sure you use get_fs/set_fs, since vfs_read won't accept a kernel pointer otherwise. If performance is more important or you really do only have access to an inode, you can read from the page cache directly using inode-i_mapping and read_cache_page. This has the advantage that you don't need to copy the data to access it, but the disadvantage that it is more complex and can be tricky to get right. -Jeff -- Jeff Mahoney SuSE Labs - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Access content of file via inodes
Kathy KN (HK) wrote: Hi Kathy - If performance is more important or you really do only have access to an inode, you can read from the page cache directly using inode-i_mapping and read_cache_page. This has the advantage that you don't need to copy the data to access it, but the disadvantage that it is more complex and can be tricky to get right. Hi Jeff, I felt that the second suggestion seems to be more of an elegant solution, though I need to find out how to actually do it correctly. Thanks for the tip. If you have example code, that would be splendid. It's not the prettiest, and in fact I'm in the process of reworking it, but code similar to what you're looking at implementing can be found in fs/reiserfs/xattr.c; Start your analysis at reiserfs_xattr_get(). -Jeff -- Jeff Mahoney SuSE Labs - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
