This small patchset tries to solve error management in udf
unicode and callsites.
Thanks a lot to Jan Kara for all suggestions and code reviewing !
V2:
-Cc to lkml
-Patch 2 and 8 updated (See commit log)
-Adding patch 9
Of course this patchset would need extended testing.
Fabian Frederick (9):
udf: udf_get_filename(): return -ENOMEM when allocation fails
udf: remove unneccessary test in udf_build_ustr_exact()
udf: unicode: update function name in comments
udf: improve error management in udf_CS0toUTF8()
udf: improve error management in udf_CS0toNLS()
udf: bug on exotic flag in udf_get_filename()
udf: return error when newIndex is 0 in udf_translate_to_linux()
udf: propagate udf_get_filename() errors
udf: add function definition for udf_find_entry()
fs/udf/dir.c | 2 +-
fs/udf/namei.c | 85 +++++++++++++++++++++++++++++++++++++++++++-------------
fs/udf/super.c | 23 +++++++++------
fs/udf/symlink.c | 3 ++
fs/udf/unicode.c | 52 +++++++++++++++++-----------------
5 files changed, 109 insertions(+), 56 deletions(-)
--
1.9.1
udf_pc_to_char() now returns error accordingly.
udf_readdir() and udf_find_entry() process is done on
positive result.
Signed-off-by: Fabian Frederick <[email protected]>
---
fs/udf/dir.c | 2 +-
fs/udf/namei.c | 3 ++-
fs/udf/symlink.c | 3 +++
fs/udf/unicode.c | 12 +++++++-----
4 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/fs/udf/dir.c b/fs/udf/dir.c
index 541a12b..fcf227e 100644
--- a/fs/udf/dir.c
+++ b/fs/udf/dir.c
@@ -168,7 +168,7 @@ static int udf_readdir(struct file *file, struct dir_context *ctx)
}
flen = udf_get_filename(sb, nameptr, lfi, fname, UDF_NAME_LEN);
- if (!flen)
+ if (flen <= 0)
continue;
tloc = lelb_to_cpu(cfi.icb.extLocation);
diff --git a/fs/udf/namei.c b/fs/udf/namei.c
index fbf3d90..59b340c 100644
--- a/fs/udf/namei.c
+++ b/fs/udf/namei.c
@@ -234,7 +234,8 @@ static struct fileIdentDesc *udf_find_entry(struct inode *dir,
continue;
flen = udf_get_filename(sb, nameptr, lfi, fname, UDF_NAME_LEN);
- if (flen && udf_match(flen, fname, child->len, child->name))
+ if ((flen > 0) && udf_match(flen, fname, child->len,
+ child->name))
goto out_ok;
}
diff --git a/fs/udf/symlink.c b/fs/udf/symlink.c
index 8dfbc40..862535b 100644
--- a/fs/udf/symlink.c
+++ b/fs/udf/symlink.c
@@ -82,6 +82,9 @@ static int udf_pc_to_char(struct super_block *sb, unsigned char *from,
comp_len = udf_get_filename(sb, pc->componentIdent,
pc->lengthComponentIdent,
p, tolen);
+ if (comp_len < 0)
+ return comp_len;
+
p += comp_len;
tolen -= comp_len;
if (tolen == 0)
diff --git a/fs/udf/unicode.c b/fs/udf/unicode.c
index b84fee3..4911c1d 100644
--- a/fs/udf/unicode.c
+++ b/fs/udf/unicode.c
@@ -338,15 +338,17 @@ int udf_get_filename(struct super_block *sb, uint8_t *sname, int slen,
uint8_t *dname, int dlen)
{
struct ustr *filename, *unifilename;
- int len = 0;
+ int ret = 0;
filename = kmalloc(sizeof(struct ustr), GFP_NOFS);
if (!filename)
- return 0;
+ return -ENOMEM;
unifilename = kmalloc(sizeof(struct ustr), GFP_NOFS);
- if (!unifilename)
+ if (!unifilename) {
+ ret = -ENOMEM;
goto out1;
+ }
if (udf_build_ustr_exact(unifilename, sname, slen))
goto out2;
@@ -367,14 +369,14 @@ int udf_get_filename(struct super_block *sb, uint8_t *sname, int slen,
} else
goto out2;
- len = udf_translate_to_linux(dname, dlen,
+ ret = udf_translate_to_linux(dname, dlen,
filename->u_name, filename->u_len,
unifilename->u_name, unifilename->u_len);
out2:
kfree(unifilename);
out1:
kfree(filename);
- return len;
+ return ret;
}
int udf_put_filename(struct super_block *sb, const uint8_t *sname,
--
1.9.1
We can remove parameter checks:
udf_build_ustr_exact() is only called by udf_get_filename()
which now assures dest is not NULL
udf_find_entry() and udf_readdir() call udf_get_filename()
after checking sname
udf_symlink_filler() calls udf_pc_to_char() with sname=kmap(page).
udf_find_entry() and udf_readdir() call udf_get_filename with UDF_NAME_LEN
udf_pc_to_char() with PAGE_SIZE
Suggested-by: Jan Kara <[email protected]>
Signed-off-by: Fabian Frederick <[email protected]>
---
V2: Return -EIO if slen = 0 in udf_get_filename()
(suggested by Jan Kara)
fs/udf/unicode.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/fs/udf/unicode.c b/fs/udf/unicode.c
index 4911c1d..e2c079a 100644
--- a/fs/udf/unicode.c
+++ b/fs/udf/unicode.c
@@ -68,17 +68,12 @@ int udf_build_ustr(struct ustr *dest, dstring *ptr, int size)
/*
* udf_build_ustr_exact
*/
-static int udf_build_ustr_exact(struct ustr *dest, dstring *ptr, int exactsize)
+static void udf_build_ustr_exact(struct ustr *dest, dstring *ptr, int exactsize)
{
- if ((!dest) || (!ptr) || (!exactsize))
- return -1;
-
memset(dest, 0, sizeof(struct ustr));
dest->u_cmpID = ptr[0];
dest->u_len = exactsize - 1;
memcpy(dest->u_name, ptr + 1, exactsize - 1);
-
- return 0;
}
/*
@@ -340,6 +335,9 @@ int udf_get_filename(struct super_block *sb, uint8_t *sname, int slen,
struct ustr *filename, *unifilename;
int ret = 0;
+ if (!slen)
+ return -EIO;
+
filename = kmalloc(sizeof(struct ustr), GFP_NOFS);
if (!filename)
return -ENOMEM;
@@ -350,9 +348,7 @@ int udf_get_filename(struct super_block *sb, uint8_t *sname, int slen,
goto out1;
}
- if (udf_build_ustr_exact(unifilename, sname, slen))
- goto out2;
-
+ udf_build_ustr_exact(unifilename, sname, slen);
if (UDF_QUERY_FLAG(sb, UDF_FLAG_UTF8)) {
if (!udf_CS0toUTF8(filename, unifilename)) {
udf_debug("Failed in udf_get_filename: sname = %s\n",
--
1.9.1
Signed-off-by: Fabian Frederick <[email protected]>
---
fs/udf/unicode.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/udf/unicode.c b/fs/udf/unicode.c
index e2c079a..41c3bef 100644
--- a/fs/udf/unicode.c
+++ b/fs/udf/unicode.c
@@ -77,7 +77,7 @@ static void udf_build_ustr_exact(struct ustr *dest, dstring *ptr, int exactsize)
}
/*
- * udf_ocu_to_utf8
+ * udf_CS0toUTF8
*
* PURPOSE
* Convert OSTA Compressed Unicode to the UTF-8 equivalent.
@@ -149,7 +149,7 @@ int udf_CS0toUTF8(struct ustr *utf_o, const struct ustr *ocu_i)
/*
*
- * udf_utf8_to_ocu
+ * udf_UTF8toCS0
*
* PURPOSE
* Convert UTF-8 to the OSTA Compressed Unicode equivalent.
--
1.9.1
udf_CS0toUTF8() now returns -EINVAL on error.
udf_load_pvoldesc() and udf_get_filename() do the same.
Suggested-by: Jan Kara <[email protected]>
Signed-off-by: Fabian Frederick <[email protected]>
---
fs/udf/super.c | 23 ++++++++++++++---------
fs/udf/unicode.c | 9 +++++----
2 files changed, 19 insertions(+), 13 deletions(-)
diff --git a/fs/udf/super.c b/fs/udf/super.c
index 6299f34..c6a8f5f 100644
--- a/fs/udf/super.c
+++ b/fs/udf/super.c
@@ -927,17 +927,22 @@ static int udf_load_pvoldesc(struct super_block *sb, sector_t block)
#endif
}
- if (!udf_build_ustr(instr, pvoldesc->volIdent, 32))
- if (udf_CS0toUTF8(outstr, instr)) {
- strncpy(UDF_SB(sb)->s_volume_ident, outstr->u_name,
- outstr->u_len > 31 ? 31 : outstr->u_len);
- udf_debug("volIdent[] = '%s'\n",
- UDF_SB(sb)->s_volume_ident);
- }
+ if (!udf_build_ustr(instr, pvoldesc->volIdent, 32)) {
+ ret = udf_CS0toUTF8(outstr, instr);
+ if (ret < 0)
+ goto out_bh;
+
+ strncpy(UDF_SB(sb)->s_volume_ident, outstr->u_name,
+ outstr->u_len > 31 ? 31 : outstr->u_len);
+ udf_debug("volIdent[] = '%s'\n", UDF_SB(sb)->s_volume_ident);
+ }
if (!udf_build_ustr(instr, pvoldesc->volSetIdent, 128))
- if (udf_CS0toUTF8(outstr, instr))
- udf_debug("volSetIdent[] = '%s'\n", outstr->u_name);
+ ret = udf_CS0toUTF8(outstr, instr);
+ if (ret < 0)
+ goto out_bh;
+
+ udf_debug("volSetIdent[] = '%s'\n", outstr->u_name);
ret = 0;
out_bh:
diff --git a/fs/udf/unicode.c b/fs/udf/unicode.c
index 41c3bef..9008a36 100644
--- a/fs/udf/unicode.c
+++ b/fs/udf/unicode.c
@@ -89,7 +89,7 @@ static void udf_build_ustr_exact(struct ustr *dest, dstring *ptr, int exactsize)
* both of type "struct ustr *"
*
* POST-CONDITIONS
- * <return> Zero on success.
+ * <return> >= 0 on success.
*
* HISTORY
* November 12, 1997 - Andrew E. Mileski
@@ -104,7 +104,7 @@ int udf_CS0toUTF8(struct ustr *utf_o, const struct ustr *ocu_i)
ocu_len = ocu_i->u_len;
if (ocu_len == 0) {
memset(utf_o, 0, sizeof(struct ustr));
- return 0;
+ return -EINVAL;
}
cmp_id = ocu_i->u_cmpID;
@@ -112,7 +112,7 @@ int udf_CS0toUTF8(struct ustr *utf_o, const struct ustr *ocu_i)
memset(utf_o, 0, sizeof(struct ustr));
pr_err("unknown compression code (%d) stri=%s\n",
cmp_id, ocu_i->u_name);
- return 0;
+ return -EINVAL;
}
ocu = ocu_i->u_name;
@@ -350,7 +350,8 @@ int udf_get_filename(struct super_block *sb, uint8_t *sname, int slen,
udf_build_ustr_exact(unifilename, sname, slen);
if (UDF_QUERY_FLAG(sb, UDF_FLAG_UTF8)) {
- if (!udf_CS0toUTF8(filename, unifilename)) {
+ ret = udf_CS0toUTF8(filename, unifilename);
+ if (ret < 0) {
udf_debug("Failed in udf_get_filename: sname = %s\n",
sname);
goto out2;
--
1.9.1
Only callsite udf_get_filename() now returns
error code as well.
Suggested-by: Jan Kara <[email protected]>
Signed-off-by: Fabian Frederick <[email protected]>
---
fs/udf/unicode.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/fs/udf/unicode.c b/fs/udf/unicode.c
index 9008a36..488a838 100644
--- a/fs/udf/unicode.c
+++ b/fs/udf/unicode.c
@@ -257,7 +257,7 @@ static int udf_CS0toNLS(struct nls_table *nls, struct ustr *utf_o,
ocu_len = ocu_i->u_len;
if (ocu_len == 0) {
memset(utf_o, 0, sizeof(struct ustr));
- return 0;
+ return -EINVAL;
}
cmp_id = ocu_i->u_cmpID;
@@ -265,7 +265,7 @@ static int udf_CS0toNLS(struct nls_table *nls, struct ustr *utf_o,
memset(utf_o, 0, sizeof(struct ustr));
pr_err("unknown compression code (%d) stri=%s\n",
cmp_id, ocu_i->u_name);
- return 0;
+ return -EINVAL;
}
ocu = ocu_i->u_name;
@@ -357,8 +357,9 @@ int udf_get_filename(struct super_block *sb, uint8_t *sname, int slen,
goto out2;
}
} else if (UDF_QUERY_FLAG(sb, UDF_FLAG_NLS_MAP)) {
- if (!udf_CS0toNLS(UDF_SB(sb)->s_nls_map, filename,
- unifilename)) {
+ ret = udf_CS0toNLS(UDF_SB(sb)->s_nls_map, filename,
+ unifilename);
+ if (ret < 0) {
udf_debug("Failed in udf_get_filename: sname = %s\n",
sname);
goto out2;
--
1.9.1
UDF volume is only mounted with UDF_FLAG_UTF8
or UDF_FLAG_NLS_MAP (see fill udf_fill_super().
BUG() if we have something different in udf_get_filename()
Suggested-by: Jan Kara <[email protected]>
Signed-off-by: Fabian Frederick <[email protected]>
---
fs/udf/unicode.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/udf/unicode.c b/fs/udf/unicode.c
index 488a838..f37123b 100644
--- a/fs/udf/unicode.c
+++ b/fs/udf/unicode.c
@@ -365,7 +365,7 @@ int udf_get_filename(struct super_block *sb, uint8_t *sname, int slen,
goto out2;
}
} else
- goto out2;
+ BUG();
ret = udf_translate_to_linux(dname, dlen,
filename->u_name, filename->u_len,
--
1.9.1
udf_get_filename() and its callsites considered 0 as an error
without propagating an error value.
udf_translate_to_linux() now returns -EINVAL when newIndex is 0.
other functions are updated accordingly.
Signed-off-by: Fabian Frederick <[email protected]>
---
fs/udf/dir.c | 2 +-
fs/udf/namei.c | 2 +-
fs/udf/unicode.c | 4 ++--
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/fs/udf/dir.c b/fs/udf/dir.c
index fcf227e..541d9c6 100644
--- a/fs/udf/dir.c
+++ b/fs/udf/dir.c
@@ -168,7 +168,7 @@ static int udf_readdir(struct file *file, struct dir_context *ctx)
}
flen = udf_get_filename(sb, nameptr, lfi, fname, UDF_NAME_LEN);
- if (flen <= 0)
+ if (flen < 0)
continue;
tloc = lelb_to_cpu(cfi.icb.extLocation);
diff --git a/fs/udf/namei.c b/fs/udf/namei.c
index 59b340c..dd648b7 100644
--- a/fs/udf/namei.c
+++ b/fs/udf/namei.c
@@ -234,7 +234,7 @@ 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,
+ if ((flen >= 0) && udf_match(flen, fname, child->len,
child->name))
goto out_ok;
}
diff --git a/fs/udf/unicode.c b/fs/udf/unicode.c
index f37123b..3b1efbb 100644
--- a/fs/udf/unicode.c
+++ b/fs/udf/unicode.c
@@ -333,7 +333,7 @@ int udf_get_filename(struct super_block *sb, uint8_t *sname, int slen,
uint8_t *dname, int dlen)
{
struct ustr *filename, *unifilename;
- int ret = 0;
+ int ret;
if (!slen)
return -EIO;
@@ -492,5 +492,5 @@ static int udf_translate_to_linux(uint8_t *newName, int newLen,
}
}
- return newIndex;
+ return newIndex ? : -EINVAL;
}
--
1.9.1
-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 <[email protected]>
Signed-off-by: Fabian Frederick <[email protected]>
---
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
Function return changed lately.
Suggested-by: Jan Kara <[email protected]>
Signed-off-by: Fabian Frederick <[email protected]>
---
fs/udf/namei.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/fs/udf/namei.c b/fs/udf/namei.c
index 891c067..1f8e4d0 100644
--- a/fs/udf/namei.c
+++ b/fs/udf/namei.c
@@ -138,6 +138,18 @@ int udf_write_fi(struct inode *inode, struct fileIdentDesc *cfi,
return 0;
}
+/**
+ * udf_find_entry - find entry in given directory.
+ *
+ * @dir: directory inode
+ * @child: qstr of the name
+ * @fibh: udf file identifier buffer head
+ * @cfi: current file identifier descriptor
+ *
+ * Return pointer to file identifier, 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,
--
1.9.1
On Wed 08-04-15 21:23:51, Fabian Frederick wrote:
> udf_pc_to_char() now returns error accordingly.
> udf_readdir() and udf_find_entry() process is done on
> positive result.
Thanks. Added to my tree with improved changelog.
Honza
>
> Signed-off-by: Fabian Frederick <[email protected]>
> ---
> fs/udf/dir.c | 2 +-
> fs/udf/namei.c | 3 ++-
> fs/udf/symlink.c | 3 +++
> fs/udf/unicode.c | 12 +++++++-----
> 4 files changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/fs/udf/dir.c b/fs/udf/dir.c
> index 541a12b..fcf227e 100644
> --- a/fs/udf/dir.c
> +++ b/fs/udf/dir.c
> @@ -168,7 +168,7 @@ static int udf_readdir(struct file *file, struct dir_context *ctx)
> }
>
> flen = udf_get_filename(sb, nameptr, lfi, fname, UDF_NAME_LEN);
> - if (!flen)
> + if (flen <= 0)
> continue;
>
> tloc = lelb_to_cpu(cfi.icb.extLocation);
> diff --git a/fs/udf/namei.c b/fs/udf/namei.c
> index fbf3d90..59b340c 100644
> --- a/fs/udf/namei.c
> +++ b/fs/udf/namei.c
> @@ -234,7 +234,8 @@ static struct fileIdentDesc *udf_find_entry(struct inode *dir,
> continue;
>
> flen = udf_get_filename(sb, nameptr, lfi, fname, UDF_NAME_LEN);
> - if (flen && udf_match(flen, fname, child->len, child->name))
> + if ((flen > 0) && udf_match(flen, fname, child->len,
> + child->name))
> goto out_ok;
> }
>
> diff --git a/fs/udf/symlink.c b/fs/udf/symlink.c
> index 8dfbc40..862535b 100644
> --- a/fs/udf/symlink.c
> +++ b/fs/udf/symlink.c
> @@ -82,6 +82,9 @@ static int udf_pc_to_char(struct super_block *sb, unsigned char *from,
> comp_len = udf_get_filename(sb, pc->componentIdent,
> pc->lengthComponentIdent,
> p, tolen);
> + if (comp_len < 0)
> + return comp_len;
> +
> p += comp_len;
> tolen -= comp_len;
> if (tolen == 0)
> diff --git a/fs/udf/unicode.c b/fs/udf/unicode.c
> index b84fee3..4911c1d 100644
> --- a/fs/udf/unicode.c
> +++ b/fs/udf/unicode.c
> @@ -338,15 +338,17 @@ int udf_get_filename(struct super_block *sb, uint8_t *sname, int slen,
> uint8_t *dname, int dlen)
> {
> struct ustr *filename, *unifilename;
> - int len = 0;
> + int ret = 0;
>
> filename = kmalloc(sizeof(struct ustr), GFP_NOFS);
> if (!filename)
> - return 0;
> + return -ENOMEM;
>
> unifilename = kmalloc(sizeof(struct ustr), GFP_NOFS);
> - if (!unifilename)
> + if (!unifilename) {
> + ret = -ENOMEM;
> goto out1;
> + }
>
> if (udf_build_ustr_exact(unifilename, sname, slen))
> goto out2;
> @@ -367,14 +369,14 @@ int udf_get_filename(struct super_block *sb, uint8_t *sname, int slen,
> } else
> goto out2;
>
> - len = udf_translate_to_linux(dname, dlen,
> + ret = udf_translate_to_linux(dname, dlen,
> filename->u_name, filename->u_len,
> unifilename->u_name, unifilename->u_len);
> out2:
> kfree(unifilename);
> out1:
> kfree(filename);
> - return len;
> + return ret;
> }
>
> int udf_put_filename(struct super_block *sb, const uint8_t *sname,
> --
> 1.9.1
>
--
Jan Kara <[email protected]>
SUSE Labs, CR
On Wed 08-04-15 21:23:52, Fabian Frederick wrote:
> We can remove parameter checks:
>
> udf_build_ustr_exact() is only called by udf_get_filename()
> which now assures dest is not NULL
>
> udf_find_entry() and udf_readdir() call udf_get_filename()
> after checking sname
> udf_symlink_filler() calls udf_pc_to_char() with sname=kmap(page).
>
> udf_find_entry() and udf_readdir() call udf_get_filename with UDF_NAME_LEN
> udf_pc_to_char() with PAGE_SIZE
>
> Suggested-by: Jan Kara <[email protected]>
> Signed-off-by: Fabian Frederick <[email protected]>
Thanks. Added to my tree.
Honza
> ---
> V2: Return -EIO if slen = 0 in udf_get_filename()
> (suggested by Jan Kara)
>
> fs/udf/unicode.c | 14 +++++---------
> 1 file changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/fs/udf/unicode.c b/fs/udf/unicode.c
> index 4911c1d..e2c079a 100644
> --- a/fs/udf/unicode.c
> +++ b/fs/udf/unicode.c
> @@ -68,17 +68,12 @@ int udf_build_ustr(struct ustr *dest, dstring *ptr, int size)
> /*
> * udf_build_ustr_exact
> */
> -static int udf_build_ustr_exact(struct ustr *dest, dstring *ptr, int exactsize)
> +static void udf_build_ustr_exact(struct ustr *dest, dstring *ptr, int exactsize)
> {
> - if ((!dest) || (!ptr) || (!exactsize))
> - return -1;
> -
> memset(dest, 0, sizeof(struct ustr));
> dest->u_cmpID = ptr[0];
> dest->u_len = exactsize - 1;
> memcpy(dest->u_name, ptr + 1, exactsize - 1);
> -
> - return 0;
> }
>
> /*
> @@ -340,6 +335,9 @@ int udf_get_filename(struct super_block *sb, uint8_t *sname, int slen,
> struct ustr *filename, *unifilename;
> int ret = 0;
>
> + if (!slen)
> + return -EIO;
> +
> filename = kmalloc(sizeof(struct ustr), GFP_NOFS);
> if (!filename)
> return -ENOMEM;
> @@ -350,9 +348,7 @@ int udf_get_filename(struct super_block *sb, uint8_t *sname, int slen,
> goto out1;
> }
>
> - if (udf_build_ustr_exact(unifilename, sname, slen))
> - goto out2;
> -
> + udf_build_ustr_exact(unifilename, sname, slen);
> if (UDF_QUERY_FLAG(sb, UDF_FLAG_UTF8)) {
> if (!udf_CS0toUTF8(filename, unifilename)) {
> udf_debug("Failed in udf_get_filename: sname = %s\n",
> --
> 1.9.1
>
--
Jan Kara <[email protected]>
SUSE Labs, CR
On Wed 08-04-15 21:23:53, Fabian Frederick wrote:
> Signed-off-by: Fabian Frederick <[email protected]>
Thanks. Applied.
Honza
> ---
> fs/udf/unicode.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/udf/unicode.c b/fs/udf/unicode.c
> index e2c079a..41c3bef 100644
> --- a/fs/udf/unicode.c
> +++ b/fs/udf/unicode.c
> @@ -77,7 +77,7 @@ static void udf_build_ustr_exact(struct ustr *dest, dstring *ptr, int exactsize)
> }
>
> /*
> - * udf_ocu_to_utf8
> + * udf_CS0toUTF8
> *
> * PURPOSE
> * Convert OSTA Compressed Unicode to the UTF-8 equivalent.
> @@ -149,7 +149,7 @@ int udf_CS0toUTF8(struct ustr *utf_o, const struct ustr *ocu_i)
>
> /*
> *
> - * udf_utf8_to_ocu
> + * udf_UTF8toCS0
> *
> * PURPOSE
> * Convert UTF-8 to the OSTA Compressed Unicode equivalent.
> --
> 1.9.1
>
--
Jan Kara <[email protected]>
SUSE Labs, CR
On Wed 08-04-15 21:23:54, Fabian Frederick wrote:
> udf_CS0toUTF8() now returns -EINVAL on error.
> udf_load_pvoldesc() and udf_get_filename() do the same.
>
> Suggested-by: Jan Kara <[email protected]>
> Signed-off-by: Fabian Frederick <[email protected]>
After some thought it's probably better API that udf_CS0toUTF8() simply
returns 0 when 0-lenght string is provided as an input (there's nothing
invalid in that from the point of view of that conversion functions). We
cannot actually ever call udf_CS0toUTF8() with such string currently but I
think it's better for future. So I've changed this and merged the patch.
Honza
> ---
> fs/udf/super.c | 23 ++++++++++++++---------
> fs/udf/unicode.c | 9 +++++----
> 2 files changed, 19 insertions(+), 13 deletions(-)
>
> diff --git a/fs/udf/super.c b/fs/udf/super.c
> index 6299f34..c6a8f5f 100644
> --- a/fs/udf/super.c
> +++ b/fs/udf/super.c
> @@ -927,17 +927,22 @@ static int udf_load_pvoldesc(struct super_block *sb, sector_t block)
> #endif
> }
>
> - if (!udf_build_ustr(instr, pvoldesc->volIdent, 32))
> - if (udf_CS0toUTF8(outstr, instr)) {
> - strncpy(UDF_SB(sb)->s_volume_ident, outstr->u_name,
> - outstr->u_len > 31 ? 31 : outstr->u_len);
> - udf_debug("volIdent[] = '%s'\n",
> - UDF_SB(sb)->s_volume_ident);
> - }
> + if (!udf_build_ustr(instr, pvoldesc->volIdent, 32)) {
> + ret = udf_CS0toUTF8(outstr, instr);
> + if (ret < 0)
> + goto out_bh;
> +
> + strncpy(UDF_SB(sb)->s_volume_ident, outstr->u_name,
> + outstr->u_len > 31 ? 31 : outstr->u_len);
> + udf_debug("volIdent[] = '%s'\n", UDF_SB(sb)->s_volume_ident);
> + }
>
> if (!udf_build_ustr(instr, pvoldesc->volSetIdent, 128))
> - if (udf_CS0toUTF8(outstr, instr))
> - udf_debug("volSetIdent[] = '%s'\n", outstr->u_name);
> + ret = udf_CS0toUTF8(outstr, instr);
> + if (ret < 0)
> + goto out_bh;
> +
> + udf_debug("volSetIdent[] = '%s'\n", outstr->u_name);
>
> ret = 0;
> out_bh:
> diff --git a/fs/udf/unicode.c b/fs/udf/unicode.c
> index 41c3bef..9008a36 100644
> --- a/fs/udf/unicode.c
> +++ b/fs/udf/unicode.c
> @@ -89,7 +89,7 @@ static void udf_build_ustr_exact(struct ustr *dest, dstring *ptr, int exactsize)
> * both of type "struct ustr *"
> *
> * POST-CONDITIONS
> - * <return> Zero on success.
> + * <return> >= 0 on success.
> *
> * HISTORY
> * November 12, 1997 - Andrew E. Mileski
> @@ -104,7 +104,7 @@ int udf_CS0toUTF8(struct ustr *utf_o, const struct ustr *ocu_i)
> ocu_len = ocu_i->u_len;
> if (ocu_len == 0) {
> memset(utf_o, 0, sizeof(struct ustr));
> - return 0;
> + return -EINVAL;
> }
>
> cmp_id = ocu_i->u_cmpID;
> @@ -112,7 +112,7 @@ int udf_CS0toUTF8(struct ustr *utf_o, const struct ustr *ocu_i)
> memset(utf_o, 0, sizeof(struct ustr));
> pr_err("unknown compression code (%d) stri=%s\n",
> cmp_id, ocu_i->u_name);
> - return 0;
> + return -EINVAL;
> }
>
> ocu = ocu_i->u_name;
> @@ -350,7 +350,8 @@ int udf_get_filename(struct super_block *sb, uint8_t *sname, int slen,
>
> udf_build_ustr_exact(unifilename, sname, slen);
> if (UDF_QUERY_FLAG(sb, UDF_FLAG_UTF8)) {
> - if (!udf_CS0toUTF8(filename, unifilename)) {
> + ret = udf_CS0toUTF8(filename, unifilename);
> + if (ret < 0) {
> udf_debug("Failed in udf_get_filename: sname = %s\n",
> sname);
> goto out2;
> --
> 1.9.1
>
--
Jan Kara <[email protected]>
SUSE Labs, CR
On Wed 08-04-15 21:23:55, Fabian Frederick wrote:
> Only callsite udf_get_filename() now returns
> error code as well.
Did same modification as in the previous patch. Otherwise OK, so I've
merged the patch.
Honza
>
> Suggested-by: Jan Kara <[email protected]>
> Signed-off-by: Fabian Frederick <[email protected]>
> ---
> fs/udf/unicode.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/fs/udf/unicode.c b/fs/udf/unicode.c
> index 9008a36..488a838 100644
> --- a/fs/udf/unicode.c
> +++ b/fs/udf/unicode.c
> @@ -257,7 +257,7 @@ static int udf_CS0toNLS(struct nls_table *nls, struct ustr *utf_o,
> ocu_len = ocu_i->u_len;
> if (ocu_len == 0) {
> memset(utf_o, 0, sizeof(struct ustr));
> - return 0;
> + return -EINVAL;
> }
>
> cmp_id = ocu_i->u_cmpID;
> @@ -265,7 +265,7 @@ static int udf_CS0toNLS(struct nls_table *nls, struct ustr *utf_o,
> memset(utf_o, 0, sizeof(struct ustr));
> pr_err("unknown compression code (%d) stri=%s\n",
> cmp_id, ocu_i->u_name);
> - return 0;
> + return -EINVAL;
> }
>
> ocu = ocu_i->u_name;
> @@ -357,8 +357,9 @@ int udf_get_filename(struct super_block *sb, uint8_t *sname, int slen,
> goto out2;
> }
> } else if (UDF_QUERY_FLAG(sb, UDF_FLAG_NLS_MAP)) {
> - if (!udf_CS0toNLS(UDF_SB(sb)->s_nls_map, filename,
> - unifilename)) {
> + ret = udf_CS0toNLS(UDF_SB(sb)->s_nls_map, filename,
> + unifilename);
> + if (ret < 0) {
> udf_debug("Failed in udf_get_filename: sname = %s\n",
> sname);
> goto out2;
> --
> 1.9.1
>
--
Jan Kara <[email protected]>
SUSE Labs, CR
On Wed 08-04-15 21:23:56, Fabian Frederick wrote:
> UDF volume is only mounted with UDF_FLAG_UTF8
> or UDF_FLAG_NLS_MAP (see fill udf_fill_super().
> BUG() if we have something different in udf_get_filename()
>
> Suggested-by: Jan Kara <[email protected]>
> Signed-off-by: Fabian Frederick <[email protected]>
Thanks. Merged.
Honza
> ---
> fs/udf/unicode.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/udf/unicode.c b/fs/udf/unicode.c
> index 488a838..f37123b 100644
> --- a/fs/udf/unicode.c
> +++ b/fs/udf/unicode.c
> @@ -365,7 +365,7 @@ int udf_get_filename(struct super_block *sb, uint8_t *sname, int slen,
> goto out2;
> }
> } else
> - goto out2;
> + BUG();
>
> ret = udf_translate_to_linux(dname, dlen,
> filename->u_name, filename->u_len,
> --
> 1.9.1
>
--
Jan Kara <[email protected]>
SUSE Labs, CR
On Wed 08-04-15 21:23:57, Fabian Frederick wrote:
> udf_get_filename() and its callsites considered 0 as an error
> without propagating an error value.
>
> udf_translate_to_linux() now returns -EINVAL when newIndex is 0.
> other functions are updated accordingly.
>
> Signed-off-by: Fabian Frederick <[email protected]>
I've updated modified this patch and the changelog to do the check for
zero length filename in udf_get_filename() and not in
udf_translate_to_linux(). Since the second is just a general conversion
function and zero length string as a result is fine with that. But zero
length file name isn't correct and that's why udf_get_filename() should
generate the error. I've also updated the changelog accordingly.
Honza
> ---
> fs/udf/dir.c | 2 +-
> fs/udf/namei.c | 2 +-
> fs/udf/unicode.c | 4 ++--
> 3 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/fs/udf/dir.c b/fs/udf/dir.c
> index fcf227e..541d9c6 100644
> --- a/fs/udf/dir.c
> +++ b/fs/udf/dir.c
> @@ -168,7 +168,7 @@ static int udf_readdir(struct file *file, struct dir_context *ctx)
> }
>
> flen = udf_get_filename(sb, nameptr, lfi, fname, UDF_NAME_LEN);
> - if (flen <= 0)
> + if (flen < 0)
> continue;
>
> tloc = lelb_to_cpu(cfi.icb.extLocation);
> diff --git a/fs/udf/namei.c b/fs/udf/namei.c
> index 59b340c..dd648b7 100644
> --- a/fs/udf/namei.c
> +++ b/fs/udf/namei.c
> @@ -234,7 +234,7 @@ 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,
> + if ((flen >= 0) && udf_match(flen, fname, child->len,
> child->name))
> goto out_ok;
> }
> diff --git a/fs/udf/unicode.c b/fs/udf/unicode.c
> index f37123b..3b1efbb 100644
> --- a/fs/udf/unicode.c
> +++ b/fs/udf/unicode.c
> @@ -333,7 +333,7 @@ int udf_get_filename(struct super_block *sb, uint8_t *sname, int slen,
> uint8_t *dname, int dlen)
> {
> struct ustr *filename, *unifilename;
> - int ret = 0;
> + int ret;
>
> if (!slen)
> return -EIO;
> @@ -492,5 +492,5 @@ static int udf_translate_to_linux(uint8_t *newName, int newLen,
> }
> }
>
> - return newIndex;
> + return newIndex ? : -EINVAL;
> }
> --
> 1.9.1
>
--
Jan Kara <[email protected]>
SUSE Labs, CR
On Wed 08-04-15 21:23:59, Fabian Frederick wrote:
> Function return changed lately.
>
> Suggested-by: Jan Kara <[email protected]>
> Signed-off-by: Fabian Frederick <[email protected]>
I've significantly extended the documentation of udf_find_entry() and
folded this patch into the previous one.
Honza
> ---
> fs/udf/namei.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/fs/udf/namei.c b/fs/udf/namei.c
> index 891c067..1f8e4d0 100644
> --- a/fs/udf/namei.c
> +++ b/fs/udf/namei.c
> @@ -138,6 +138,18 @@ int udf_write_fi(struct inode *inode, struct fileIdentDesc *cfi,
> return 0;
> }
>
> +/**
> + * udf_find_entry - find entry in given directory.
> + *
> + * @dir: directory inode
> + * @child: qstr of the name
> + * @fibh: udf file identifier buffer head
> + * @cfi: current file identifier descriptor
> + *
> + * Return pointer to file identifier, 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,
> --
> 1.9.1
>
--
Jan Kara <[email protected]>
SUSE Labs, CR
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 <[email protected]>
> Signed-off-by: Fabian Frederick <[email protected]>
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 <[email protected]>
SUSE Labs, CR
On Thu 09-04-15 10:36:07, Jan Kara wrote:
> On Wed 08-04-15 21:23:54, Fabian Frederick wrote:
> > udf_CS0toUTF8() now returns -EINVAL on error.
> > udf_load_pvoldesc() and udf_get_filename() do the same.
> >
> > Suggested-by: Jan Kara <[email protected]>
> > Signed-off-by: Fabian Frederick <[email protected]>
> After some thought it's probably better API that udf_CS0toUTF8() simply
> returns 0 when 0-lenght string is provided as an input (there's nothing
> invalid in that from the point of view of that conversion functions). We
> cannot actually ever call udf_CS0toUTF8() with such string currently but I
> think it's better for future. So I've changed this and merged the patch.
Just for reference, attached is the resulting patch.
Honza
> > ---
> > fs/udf/super.c | 23 ++++++++++++++---------
> > fs/udf/unicode.c | 9 +++++----
> > 2 files changed, 19 insertions(+), 13 deletions(-)
> >
> > diff --git a/fs/udf/super.c b/fs/udf/super.c
> > index 6299f34..c6a8f5f 100644
> > --- a/fs/udf/super.c
> > +++ b/fs/udf/super.c
> > @@ -927,17 +927,22 @@ static int udf_load_pvoldesc(struct super_block *sb, sector_t block)
> > #endif
> > }
> >
> > - if (!udf_build_ustr(instr, pvoldesc->volIdent, 32))
> > - if (udf_CS0toUTF8(outstr, instr)) {
> > - strncpy(UDF_SB(sb)->s_volume_ident, outstr->u_name,
> > - outstr->u_len > 31 ? 31 : outstr->u_len);
> > - udf_debug("volIdent[] = '%s'\n",
> > - UDF_SB(sb)->s_volume_ident);
> > - }
> > + if (!udf_build_ustr(instr, pvoldesc->volIdent, 32)) {
> > + ret = udf_CS0toUTF8(outstr, instr);
> > + if (ret < 0)
> > + goto out_bh;
> > +
> > + strncpy(UDF_SB(sb)->s_volume_ident, outstr->u_name,
> > + outstr->u_len > 31 ? 31 : outstr->u_len);
> > + udf_debug("volIdent[] = '%s'\n", UDF_SB(sb)->s_volume_ident);
> > + }
> >
> > if (!udf_build_ustr(instr, pvoldesc->volSetIdent, 128))
> > - if (udf_CS0toUTF8(outstr, instr))
> > - udf_debug("volSetIdent[] = '%s'\n", outstr->u_name);
> > + ret = udf_CS0toUTF8(outstr, instr);
> > + if (ret < 0)
> > + goto out_bh;
> > +
> > + udf_debug("volSetIdent[] = '%s'\n", outstr->u_name);
> >
> > ret = 0;
> > out_bh:
> > diff --git a/fs/udf/unicode.c b/fs/udf/unicode.c
> > index 41c3bef..9008a36 100644
> > --- a/fs/udf/unicode.c
> > +++ b/fs/udf/unicode.c
> > @@ -89,7 +89,7 @@ static void udf_build_ustr_exact(struct ustr *dest, dstring *ptr, int exactsize)
> > * both of type "struct ustr *"
> > *
> > * POST-CONDITIONS
> > - * <return> Zero on success.
> > + * <return> >= 0 on success.
> > *
> > * HISTORY
> > * November 12, 1997 - Andrew E. Mileski
> > @@ -104,7 +104,7 @@ int udf_CS0toUTF8(struct ustr *utf_o, const struct ustr *ocu_i)
> > ocu_len = ocu_i->u_len;
> > if (ocu_len == 0) {
> > memset(utf_o, 0, sizeof(struct ustr));
> > - return 0;
> > + return -EINVAL;
> > }
> >
> > cmp_id = ocu_i->u_cmpID;
> > @@ -112,7 +112,7 @@ int udf_CS0toUTF8(struct ustr *utf_o, const struct ustr *ocu_i)
> > memset(utf_o, 0, sizeof(struct ustr));
> > pr_err("unknown compression code (%d) stri=%s\n",
> > cmp_id, ocu_i->u_name);
> > - return 0;
> > + return -EINVAL;
> > }
> >
> > ocu = ocu_i->u_name;
> > @@ -350,7 +350,8 @@ int udf_get_filename(struct super_block *sb, uint8_t *sname, int slen,
> >
> > udf_build_ustr_exact(unifilename, sname, slen);
> > if (UDF_QUERY_FLAG(sb, UDF_FLAG_UTF8)) {
> > - if (!udf_CS0toUTF8(filename, unifilename)) {
> > + ret = udf_CS0toUTF8(filename, unifilename);
> > + if (ret < 0) {
> > udf_debug("Failed in udf_get_filename: sname = %s\n",
> > sname);
> > goto out2;
> > --
> > 1.9.1
> >
> --
> Jan Kara <[email protected]>
> SUSE Labs, CR
--
Jan Kara <[email protected]>
SUSE Labs, CR
On Thu 09-04-15 10:37:42, Jan Kara wrote:
> On Wed 08-04-15 21:23:55, Fabian Frederick wrote:
> > Only callsite udf_get_filename() now returns
> > error code as well.
> Did same modification as in the previous patch. Otherwise OK, so I've
> merged the patch.
The resulting patch is attached for reference.
Honza
> > Suggested-by: Jan Kara <[email protected]>
> > Signed-off-by: Fabian Frederick <[email protected]>
> > ---
> > fs/udf/unicode.c | 9 +++++----
> > 1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/fs/udf/unicode.c b/fs/udf/unicode.c
> > index 9008a36..488a838 100644
> > --- a/fs/udf/unicode.c
> > +++ b/fs/udf/unicode.c
> > @@ -257,7 +257,7 @@ static int udf_CS0toNLS(struct nls_table *nls, struct ustr *utf_o,
> > ocu_len = ocu_i->u_len;
> > if (ocu_len == 0) {
> > memset(utf_o, 0, sizeof(struct ustr));
> > - return 0;
> > + return -EINVAL;
> > }
> >
> > cmp_id = ocu_i->u_cmpID;
> > @@ -265,7 +265,7 @@ static int udf_CS0toNLS(struct nls_table *nls, struct ustr *utf_o,
> > memset(utf_o, 0, sizeof(struct ustr));
> > pr_err("unknown compression code (%d) stri=%s\n",
> > cmp_id, ocu_i->u_name);
> > - return 0;
> > + return -EINVAL;
> > }
> >
> > ocu = ocu_i->u_name;
> > @@ -357,8 +357,9 @@ int udf_get_filename(struct super_block *sb, uint8_t *sname, int slen,
> > goto out2;
> > }
> > } else if (UDF_QUERY_FLAG(sb, UDF_FLAG_NLS_MAP)) {
> > - if (!udf_CS0toNLS(UDF_SB(sb)->s_nls_map, filename,
> > - unifilename)) {
> > + ret = udf_CS0toNLS(UDF_SB(sb)->s_nls_map, filename,
> > + unifilename);
> > + if (ret < 0) {
> > udf_debug("Failed in udf_get_filename: sname = %s\n",
> > sname);
> > goto out2;
> > --
> > 1.9.1
> >
> --
> Jan Kara <[email protected]>
> SUSE Labs, CR
--
Jan Kara <[email protected]>
SUSE Labs, CR
On Thu 09-04-15 10:51:05, Jan Kara wrote:
> On Wed 08-04-15 21:23:57, Fabian Frederick wrote:
> > udf_get_filename() and its callsites considered 0 as an error
> > without propagating an error value.
> >
> > udf_translate_to_linux() now returns -EINVAL when newIndex is 0.
> > other functions are updated accordingly.
> >
> > Signed-off-by: Fabian Frederick <[email protected]>
> I've updated modified this patch and the changelog to do the check for
> zero length filename in udf_get_filename() and not in
> udf_translate_to_linux(). Since the second is just a general conversion
> function and zero length string as a result is fine with that. But zero
> length file name isn't correct and that's why udf_get_filename() should
> generate the error. I've also updated the changelog accordingly.
Attached is the resulting patch for reference.
Honza
> > fs/udf/dir.c | 2 +-
> > fs/udf/namei.c | 2 +-
> > fs/udf/unicode.c | 4 ++--
> > 3 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/fs/udf/dir.c b/fs/udf/dir.c
> > index fcf227e..541d9c6 100644
> > --- a/fs/udf/dir.c
> > +++ b/fs/udf/dir.c
> > @@ -168,7 +168,7 @@ static int udf_readdir(struct file *file, struct dir_context *ctx)
> > }
> >
> > flen = udf_get_filename(sb, nameptr, lfi, fname, UDF_NAME_LEN);
> > - if (flen <= 0)
> > + if (flen < 0)
> > continue;
> >
> > tloc = lelb_to_cpu(cfi.icb.extLocation);
> > diff --git a/fs/udf/namei.c b/fs/udf/namei.c
> > index 59b340c..dd648b7 100644
> > --- a/fs/udf/namei.c
> > +++ b/fs/udf/namei.c
> > @@ -234,7 +234,7 @@ 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,
> > + if ((flen >= 0) && udf_match(flen, fname, child->len,
> > child->name))
> > goto out_ok;
> > }
> > diff --git a/fs/udf/unicode.c b/fs/udf/unicode.c
> > index f37123b..3b1efbb 100644
> > --- a/fs/udf/unicode.c
> > +++ b/fs/udf/unicode.c
> > @@ -333,7 +333,7 @@ int udf_get_filename(struct super_block *sb, uint8_t *sname, int slen,
> > uint8_t *dname, int dlen)
> > {
> > struct ustr *filename, *unifilename;
> > - int ret = 0;
> > + int ret;
> >
> > if (!slen)
> > return -EIO;
> > @@ -492,5 +492,5 @@ static int udf_translate_to_linux(uint8_t *newName, int newLen,
> > }
> > }
> >
> > - return newIndex;
> > + return newIndex ? : -EINVAL;
> > }
> > --
> > 1.9.1
> >
> --
> Jan Kara <[email protected]>
> SUSE Labs, CR
--
Jan Kara <[email protected]>
SUSE Labs, CR