lkml.org 
[lkml]   [2015]   [Apr]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH V2 8/9 linux-next] udf: propagate udf_get_filename() errors
On Wed 08-04-15 21:23:58, Fabian Frederick wrote:
> -Return udf_get_filename() error from udf_readdir()
> -Return -ENOMEM from udf_find_entry() when unable to allocate
> fname and udf_get_filename() error
> -udf_find_entry() callsites are also updated:
> udf_lookup(), udf_rmdir(), udf_unlink() and udf_rename()
>
> Suggested-by: Jan Kara <jack@suse.cz>
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
Thanks. I've merged this patch with slight modification -
udf_find_entry() should return -EIO instead of -EINVAL in all the cases.
I've also folded the documentation of udf_find_entry() into this patch.
Attached is the result.

Honza
> ---
> V2:
> -Don't set error in udf_readdir()
> -Improve code flow
> -Merge if(nfi) if (!inode) in udf_rename()
>
> fs/udf/namei.c | 70 ++++++++++++++++++++++++++++++++++++++++++----------------
> 1 file changed, 51 insertions(+), 19 deletions(-)
>
> diff --git a/fs/udf/namei.c b/fs/udf/namei.c
> index dd648b7..891c067 100644
> --- a/fs/udf/namei.c
> +++ b/fs/udf/namei.c
> @@ -167,8 +167,11 @@ static struct fileIdentDesc *udf_find_entry(struct inode *dir,
> fibh->soffset = fibh->eoffset = f_pos & (sb->s_blocksize - 1);
> if (dinfo->i_alloc_type != ICBTAG_FLAG_AD_IN_ICB) {
> if (inode_bmap(dir, f_pos >> sb->s_blocksize_bits, &epos,
> - &eloc, &elen, &offset) != (EXT_RECORDED_ALLOCATED >> 30))
> + &eloc, &elen, &offset) != (EXT_RECORDED_ALLOCATED >> 30)) {
> + fi = ERR_PTR(-EINVAL);
> goto out_err;
> + }
> +
> block = udf_get_lb_pblock(sb, &eloc, offset);
> if ((++offset << sb->s_blocksize_bits) < elen) {
> if (dinfo->i_alloc_type == ICBTAG_FLAG_AD_SHORT)
> @@ -179,19 +182,25 @@ static struct fileIdentDesc *udf_find_entry(struct inode *dir,
> offset = 0;
>
> fibh->sbh = fibh->ebh = udf_tread(sb, block);
> - if (!fibh->sbh)
> + if (!fibh->sbh) {
> + fi = ERR_PTR(-EINVAL);
> goto out_err;
> + }
> }
>
> fname = kmalloc(UDF_NAME_LEN, GFP_NOFS);
> - if (!fname)
> + if (!fname) {
> + fi = ERR_PTR(-ENOMEM);
> goto out_err;
> + }
>
> while (f_pos < size) {
> fi = udf_fileident_read(dir, &f_pos, fibh, cfi, &epos, &eloc,
> &elen, &offset);
> - if (!fi)
> + if (!fi) {
> + fi = ERR_PTR(-EINVAL);
> goto out_err;
> + }
>
> liu = le16_to_cpu(cfi->lengthOfImpUse);
> lfi = cfi->lengthFileIdent;
> @@ -234,13 +243,18 @@ static struct fileIdentDesc *udf_find_entry(struct inode *dir,
> continue;
>
> flen = udf_get_filename(sb, nameptr, lfi, fname, UDF_NAME_LEN);
> + if (flen < 0) {
> + fi = ERR_PTR(flen);
> + goto out_err;
> + }
> +
> if ((flen >= 0) && udf_match(flen, fname, child->len,
> child->name))
> goto out_ok;
> }
>
> -out_err:
> fi = NULL;
> +out_err:
> if (fibh->sbh != fibh->ebh)
> brelse(fibh->ebh);
> brelse(fibh->sbh);
> @@ -257,6 +271,7 @@ static struct dentry *udf_lookup(struct inode *dir, struct dentry *dentry,
> struct inode *inode = NULL;
> struct fileIdentDesc cfi;
> struct udf_fileident_bh fibh;
> + struct fileIdentDesc *fi;
>
> if (dentry->d_name.len > UDF_NAME_LEN - 2)
> return ERR_PTR(-ENAMETOOLONG);
> @@ -276,7 +291,11 @@ static struct dentry *udf_lookup(struct inode *dir, struct dentry *dentry,
> } else
> #endif /* UDF_RECOVERY */
>
> - if (udf_find_entry(dir, &dentry->d_name, &fibh, &cfi)) {
> + fi = udf_find_entry(dir, &dentry->d_name, &fibh, &cfi);
> + if (IS_ERR(fi))
> + return ERR_CAST(fi);
> +
> + if (fi) {
> struct kernel_lb_addr loc;
>
> if (fibh.sbh != fibh.ebh)
> @@ -774,8 +793,11 @@ static int udf_rmdir(struct inode *dir, struct dentry *dentry)
>
> retval = -ENOENT;
> fi = udf_find_entry(dir, &dentry->d_name, &fibh, &cfi);
> - if (!fi)
> + if (IS_ERR_OR_NULL(fi)) {
> + if (fi)
> + retval = PTR_ERR(fi);
> goto out;
> + }
>
> retval = -EIO;
> tloc = lelb_to_cpu(cfi.icb.extLocation);
> @@ -817,8 +839,12 @@ static int udf_unlink(struct inode *dir, struct dentry *dentry)
>
> retval = -ENOENT;
> fi = udf_find_entry(dir, &dentry->d_name, &fibh, &cfi);
> - if (!fi)
> +
> + if (IS_ERR_OR_NULL(fi)) {
> + if (fi)
> + retval = PTR_ERR(fi);
> goto out;
> + }
>
> retval = -EIO;
> tloc = lelb_to_cpu(cfi.icb.extLocation);
> @@ -1047,24 +1073,30 @@ static int udf_rename(struct inode *old_dir, struct dentry *old_dentry,
> struct udf_inode_info *old_iinfo = UDF_I(old_inode);
>
> ofi = udf_find_entry(old_dir, &old_dentry->d_name, &ofibh, &ocfi);
> - if (ofi) {
> - if (ofibh.sbh != ofibh.ebh)
> - brelse(ofibh.ebh);
> - brelse(ofibh.sbh);
> + if (IS_ERR(ofi)) {
> + retval = PTR_ERR(ofi);
> + goto end_rename;
> }
> +
> + if (ofibh.sbh != ofibh.ebh)
> + brelse(ofibh.ebh);
> +
> + brelse(ofibh.sbh);
> tloc = lelb_to_cpu(ocfi.icb.extLocation);
> if (!ofi || udf_get_lb_pblock(old_dir->i_sb, &tloc, 0)
> != old_inode->i_ino)
> goto end_rename;
>
> nfi = udf_find_entry(new_dir, &new_dentry->d_name, &nfibh, &ncfi);
> - if (nfi) {
> - if (!new_inode) {
> - if (nfibh.sbh != nfibh.ebh)
> - brelse(nfibh.ebh);
> - brelse(nfibh.sbh);
> - nfi = NULL;
> - }
> + if (IS_ERR(nfi)) {
> + retval = PTR_ERR(nfi);
> + goto end_rename;
> + }
> + if (nfi && !new_inode) {
> + if (nfibh.sbh != nfibh.ebh)
> + brelse(nfibh.ebh);
> + brelse(nfibh.sbh);
> + nfi = NULL;
> }
> if (S_ISDIR(old_inode->i_mode)) {
> int offset = udf_ext0_offset(old_inode);
> --
> 1.9.1
>
--
Jan Kara <jack@suse.cz>
SUSE Labs, CR
From d0cf0af6334553955107a9136f9da85c2e5dfd73 Mon Sep 17 00:00:00 2001
From: Fabian Frederick <fabf@skynet.be>
Date: Wed, 8 Apr 2015 21:23:58 +0200
Subject: [PATCH] udf: Return error from udf_find_entry()

Return appropriate error from udf_find_entry() instead of just NULL.
That way we can distinguish the fact that some error happened when
looking up filename (and return error to userspace) from the fact that
we just didn't find the filename. Also update callers of
udf_find_entry() accordingly.

[JK: Improved udf_find_entry() documentation]

Suggested-by: Jan Kara <jack@suse.cz>
Signed-off-by: Fabian Frederick <fabf@skynet.be>
Signed-off-by: Jan Kara <jack@suse.cz>
---
fs/udf/namei.c | 91 +++++++++++++++++++++++++++++++++++++++++++++-------------
1 file changed, 71 insertions(+), 20 deletions(-)

diff --git a/fs/udf/namei.c b/fs/udf/namei.c
index 5e833bd1c4e9..5767c36d5652 100644
--- a/fs/udf/namei.c
+++ b/fs/udf/namei.c
@@ -138,6 +138,25 @@ int udf_write_fi(struct inode *inode, struct fileIdentDesc *cfi,
return 0;
}

+/**
+ * udf_find_entry - find entry in given directory.
+ *
+ * @dir: directory inode to search in
+ * @child: qstr of the name
+ * @fibh: buffer head / inode with file identifier descriptor we found
+ * @cfi: found file identifier descriptor with given name
+ *
+ * This function searches in the directory @dir for a file name @child. When
+ * found, @fibh points to the buffer head(s) (bh is NULL for in ICB
+ * directories) containing the file identifier descriptor (FID). In that case
+ * the function returns pointer to the FID in the buffer or inode - but note
+ * that FID may be split among two buffers (blocks) so accessing it via that
+ * pointer isn't easily possible. This pointer can be used only as an iterator
+ * for other directory manipulation functions. For inspection of the FID @cfi
+ * can be used - the found FID is copied there.
+ *
+ * Returns pointer to FID, NULL when nothing found, or error code.
+ */
static struct fileIdentDesc *udf_find_entry(struct inode *dir,
const struct qstr *child,
struct udf_fileident_bh *fibh,
@@ -167,8 +186,11 @@ static struct fileIdentDesc *udf_find_entry(struct inode *dir,
fibh->soffset = fibh->eoffset = f_pos & (sb->s_blocksize - 1);
if (dinfo->i_alloc_type != ICBTAG_FLAG_AD_IN_ICB) {
if (inode_bmap(dir, f_pos >> sb->s_blocksize_bits, &epos,
- &eloc, &elen, &offset) != (EXT_RECORDED_ALLOCATED >> 30))
+ &eloc, &elen, &offset) != (EXT_RECORDED_ALLOCATED >> 30)) {
+ fi = ERR_PTR(-EIO);
goto out_err;
+ }
+
block = udf_get_lb_pblock(sb, &eloc, offset);
if ((++offset << sb->s_blocksize_bits) < elen) {
if (dinfo->i_alloc_type == ICBTAG_FLAG_AD_SHORT)
@@ -179,19 +201,25 @@ static struct fileIdentDesc *udf_find_entry(struct inode *dir,
offset = 0;

fibh->sbh = fibh->ebh = udf_tread(sb, block);
- if (!fibh->sbh)
+ if (!fibh->sbh) {
+ fi = ERR_PTR(-EIO);
goto out_err;
+ }
}

fname = kmalloc(UDF_NAME_LEN, GFP_NOFS);
- if (!fname)
+ if (!fname) {
+ fi = ERR_PTR(-ENOMEM);
goto out_err;
+ }

while (f_pos < size) {
fi = udf_fileident_read(dir, &f_pos, fibh, cfi, &epos, &eloc,
&elen, &offset);
- if (!fi)
+ if (!fi) {
+ fi = ERR_PTR(-EIO);
goto out_err;
+ }

liu = le16_to_cpu(cfi->lengthOfImpUse);
lfi = cfi->lengthFileIdent;
@@ -234,12 +262,17 @@ static struct fileIdentDesc *udf_find_entry(struct inode *dir,
continue;

flen = udf_get_filename(sb, nameptr, lfi, fname, UDF_NAME_LEN);
- if (flen > 0 && udf_match(flen, fname, child->len, child->name))
+ if (flen < 0) {
+ fi = ERR_PTR(flen);
+ goto out_err;
+ }
+
+ if (udf_match(flen, fname, child->len, child->name))
goto out_ok;
}

-out_err:
fi = NULL;
+out_err:
if (fibh->sbh != fibh->ebh)
brelse(fibh->ebh);
brelse(fibh->sbh);
@@ -256,6 +289,7 @@ static struct dentry *udf_lookup(struct inode *dir, struct dentry *dentry,
struct inode *inode = NULL;
struct fileIdentDesc cfi;
struct udf_fileident_bh fibh;
+ struct fileIdentDesc *fi;

if (dentry->d_name.len > UDF_NAME_LEN - 2)
return ERR_PTR(-ENAMETOOLONG);
@@ -275,7 +309,11 @@ static struct dentry *udf_lookup(struct inode *dir, struct dentry *dentry,
} else
#endif /* UDF_RECOVERY */

- if (udf_find_entry(dir, &dentry->d_name, &fibh, &cfi)) {
+ fi = udf_find_entry(dir, &dentry->d_name, &fibh, &cfi);
+ if (IS_ERR(fi))
+ return ERR_CAST(fi);
+
+ if (fi) {
struct kernel_lb_addr loc;

if (fibh.sbh != fibh.ebh)
@@ -774,8 +812,11 @@ static int udf_rmdir(struct inode *dir, struct dentry *dentry)

retval = -ENOENT;
fi = udf_find_entry(dir, &dentry->d_name, &fibh, &cfi);
- if (!fi)
+ if (IS_ERR_OR_NULL(fi)) {
+ if (fi)
+ retval = PTR_ERR(fi);
goto out;
+ }

retval = -EIO;
tloc = lelb_to_cpu(cfi.icb.extLocation);
@@ -817,8 +858,12 @@ static int udf_unlink(struct inode *dir, struct dentry *dentry)

retval = -ENOENT;
fi = udf_find_entry(dir, &dentry->d_name, &fibh, &cfi);
- if (!fi)
+
+ if (IS_ERR_OR_NULL(fi)) {
+ if (fi)
+ retval = PTR_ERR(fi);
goto out;
+ }

retval = -EIO;
tloc = lelb_to_cpu(cfi.icb.extLocation);
@@ -1049,24 +1094,30 @@ static int udf_rename(struct inode *old_dir, struct dentry *old_dentry,
struct udf_inode_info *old_iinfo = UDF_I(old_inode);

ofi = udf_find_entry(old_dir, &old_dentry->d_name, &ofibh, &ocfi);
- if (ofi) {
- if (ofibh.sbh != ofibh.ebh)
- brelse(ofibh.ebh);
- brelse(ofibh.sbh);
+ if (IS_ERR(ofi)) {
+ retval = PTR_ERR(ofi);
+ goto end_rename;
}
+
+ if (ofibh.sbh != ofibh.ebh)
+ brelse(ofibh.ebh);
+
+ brelse(ofibh.sbh);
tloc = lelb_to_cpu(ocfi.icb.extLocation);
if (!ofi || udf_get_lb_pblock(old_dir->i_sb, &tloc, 0)
!= old_inode->i_ino)
goto end_rename;

nfi = udf_find_entry(new_dir, &new_dentry->d_name, &nfibh, &ncfi);
- if (nfi) {
- if (!new_inode) {
- if (nfibh.sbh != nfibh.ebh)
- brelse(nfibh.ebh);
- brelse(nfibh.sbh);
- nfi = NULL;
- }
+ if (IS_ERR(nfi)) {
+ retval = PTR_ERR(nfi);
+ goto end_rename;
+ }
+ if (nfi && !new_inode) {
+ if (nfibh.sbh != nfibh.ebh)
+ brelse(nfibh.ebh);
+ brelse(nfibh.sbh);
+ nfi = NULL;
}
if (S_ISDIR(old_inode->i_mode)) {
int offset = udf_ext0_offset(old_inode);
--
2.1.4
\
 
 \ /
  Last update: 2015-04-09 12:01    [W:0.102 / U:17.336 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site