* [PATCH] isofs: validate directory records in NFS get_parent @ 2026-07-21 10:13 Yichong Chen 2026-07-27 15:22 ` Jan Kara 0 siblings, 1 reply; 5+ messages in thread From: Yichong Chen @ 2026-07-21 10:13 UTC (permalink / raw) To: Jan Kara; +Cc: linux-fsdevel, linux-kernel, Yichong Chen isofs_export_get_parent() assumes valid "." and ".." entries. A malformed image can provide an invalid length for the first entry. Validate both records before using the first length as the ".." offset. This keeps the NFS export get_parent path from accepting malformed records. Signed-off-by: Yichong Chen <chenyichong@uniontech.com> --- fs/isofs/export.c | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/fs/isofs/export.c b/fs/isofs/export.c index 78f80c1a5c54..bffec45bb274 100644 --- a/fs/isofs/export.c +++ b/fs/isofs/export.c @@ -16,6 +16,26 @@ #include "isofs.h" +static bool isofs_dir_record_valid(struct iso_directory_record *de, + unsigned long offset, + unsigned long bufsize) +{ + unsigned int len; + unsigned int name_len; + unsigned long min_len = offsetof(struct iso_directory_record, name); + + if (offset > bufsize || bufsize - offset < min_len) + return false; + + len = isonum_711(de->length); + name_len = isonum_711(de->name_len); + if (len < min_len || name_len > len - min_len) + return false; + if (len > bufsize - offset) + return false; + return true; +} + static struct dentry * isofs_export_iget(struct super_block *sb, unsigned long block, @@ -83,13 +103,21 @@ static struct dentry *isofs_export_get_parent(struct dentry *child) /* This is the "." entry. */ de = (struct iso_directory_record*)bh->b_data; + if (!isofs_dir_record_valid(de, 0, child_inode->i_sb->s_blocksize) || + isonum_711(de->name_len) != 1 || de->name[0] != 0) { + printk(KERN_ERR "isofs: Unable to find the \".\" directory for NFS.\n"); + rv = ERR_PTR(-EACCES); + goto out; + } /* The ".." entry is always the second entry. */ parent_offset = (unsigned long)isonum_711(de->length); de = (struct iso_directory_record*)(bh->b_data + parent_offset); /* Verify it is in fact the ".." entry. */ - if ((isonum_711(de->name_len) != 1) || (de->name[0] != 1)) { + if (!isofs_dir_record_valid(de, parent_offset, + child_inode->i_sb->s_blocksize) || + isonum_711(de->name_len) != 1 || de->name[0] != 1) { printk(KERN_ERR "isofs: Unable to find the \"..\" " "directory for NFS.\n"); rv = ERR_PTR(-EACCES); -- 2.51.0 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] isofs: validate directory records in NFS get_parent 2026-07-21 10:13 [PATCH] isofs: validate directory records in NFS get_parent Yichong Chen @ 2026-07-27 15:22 ` Jan Kara 2026-07-28 1:42 ` Yichong Chen 2026-07-28 7:43 ` [PATCH v2] isofs: validate directory records consistently Yichong Chen 0 siblings, 2 replies; 5+ messages in thread From: Jan Kara @ 2026-07-27 15:22 UTC (permalink / raw) To: Yichong Chen; +Cc: Jan Kara, linux-fsdevel, linux-kernel On Tue 21-07-26 18:13:18, Yichong Chen wrote: > isofs_export_get_parent() assumes valid "." and ".." entries. > > A malformed image can provide an invalid length for the first entry. > > Validate both records before using the first length as the ".." offset. > > This keeps the NFS export get_parent path from accepting malformed records. > > Signed-off-by: Yichong Chen <chenyichong@uniontech.com> This looks sensible. But for maintainability, can you please move isofs_dir_record_valid() to namei.c and make sure readdir and directory lookup functions are using this function for directory entry checking as well (and remove the duplicit checks there)? So that we have the same checks everywhere... Thanks. Honza > --- > fs/isofs/export.c | 30 +++++++++++++++++++++++++++++- > 1 file changed, 29 insertions(+), 1 deletion(-) > > diff --git a/fs/isofs/export.c b/fs/isofs/export.c > index 78f80c1a5c54..bffec45bb274 100644 > --- a/fs/isofs/export.c > +++ b/fs/isofs/export.c > @@ -16,6 +16,26 @@ > > #include "isofs.h" > > +static bool isofs_dir_record_valid(struct iso_directory_record *de, > + unsigned long offset, > + unsigned long bufsize) > +{ > + unsigned int len; > + unsigned int name_len; > + unsigned long min_len = offsetof(struct iso_directory_record, name); > + > + if (offset > bufsize || bufsize - offset < min_len) > + return false; > + > + len = isonum_711(de->length); > + name_len = isonum_711(de->name_len); > + if (len < min_len || name_len > len - min_len) > + return false; > + if (len > bufsize - offset) > + return false; > + return true; > +} > + > static struct dentry * > isofs_export_iget(struct super_block *sb, > unsigned long block, > @@ -83,13 +103,21 @@ static struct dentry *isofs_export_get_parent(struct dentry *child) > > /* This is the "." entry. */ > de = (struct iso_directory_record*)bh->b_data; > + if (!isofs_dir_record_valid(de, 0, child_inode->i_sb->s_blocksize) || > + isonum_711(de->name_len) != 1 || de->name[0] != 0) { > + printk(KERN_ERR "isofs: Unable to find the \".\" directory for NFS.\n"); > + rv = ERR_PTR(-EACCES); > + goto out; > + } > > /* The ".." entry is always the second entry. */ > parent_offset = (unsigned long)isonum_711(de->length); > de = (struct iso_directory_record*)(bh->b_data + parent_offset); > > /* Verify it is in fact the ".." entry. */ > - if ((isonum_711(de->name_len) != 1) || (de->name[0] != 1)) { > + if (!isofs_dir_record_valid(de, parent_offset, > + child_inode->i_sb->s_blocksize) || > + isonum_711(de->name_len) != 1 || de->name[0] != 1) { > printk(KERN_ERR "isofs: Unable to find the \"..\" " > "directory for NFS.\n"); > rv = ERR_PTR(-EACCES); > -- > 2.51.0 > -- Jan Kara <jack@suse.com> SUSE Labs, CR ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] isofs: validate directory records in NFS get_parent 2026-07-27 15:22 ` Jan Kara @ 2026-07-28 1:42 ` Yichong Chen 2026-07-28 7:43 ` [PATCH v2] isofs: validate directory records consistently Yichong Chen 1 sibling, 0 replies; 5+ messages in thread From: Yichong Chen @ 2026-07-28 1:42 UTC (permalink / raw) To: jack; +Cc: chenyichong, linux-fsdevel, linux-kernel Thanks for the review. I'll rework it as suggested and send a v2. Yichong ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2] isofs: validate directory records consistently 2026-07-27 15:22 ` Jan Kara 2026-07-28 1:42 ` Yichong Chen @ 2026-07-28 7:43 ` Yichong Chen 2026-07-28 16:05 ` Jan Kara 1 sibling, 1 reply; 5+ messages in thread From: Yichong Chen @ 2026-07-28 7:43 UTC (permalink / raw) To: jack; +Cc: linux-fsdevel, linux-kernel, Yichong Chen isofs_export_get_parent() assumes that the first two directory records are valid "." and ".." entries. A malformed image can provide an invalid length for the first entry, causing the computed ".." offset to point outside the received block. Add a shared directory record validator and use it in NFS get_parent, readdir and lookup. This keeps the basic directory record length checks consistent across all directory users before they consume the name field or use one record length to find the next entry. Signed-off-by: Yichong Chen <chenyichong@uniontech.com> --- v2: - Move the directory record validator to namei.c. - Share the validator with NFS get_parent, readdir and lookup. - Replace duplicate directory record length checks. fs/isofs/dir.c | 7 ++----- fs/isofs/export.c | 10 +++++++++- fs/isofs/isofs.h | 3 +++ fs/isofs/namei.c | 28 ++++++++++++++++++++++++---- 4 files changed, 38 insertions(+), 10 deletions(-) diff --git a/fs/isofs/dir.c b/fs/isofs/dir.c index cc587cd25162..a96268c9ca41 100644 --- a/fs/isofs/dir.c +++ b/fs/isofs/dir.c @@ -149,10 +149,8 @@ static int do_isofs_readdir(struct inode *inode, struct file *file, } de = tmpde; } - /* Basic sanity check, whether name doesn't exceed dir entry */ - if (de_len < sizeof(struct iso_directory_record) || - de_len < de->name_len[0] + - sizeof(struct iso_directory_record)) { + if (!isofs_dir_record_valid(de, de == tmpde ? 0 : offset_saved, + de == tmpde ? de_len : bufsize)) { printk(KERN_NOTICE "iso9660: Corrupted directory entry" " in block %lu of inode %llu\n", block, inode->i_ino); @@ -300,4 +298,3 @@ const struct inode_operations isofs_dir_inode_operations = .fileattr_get = isofs_fileattr_get, }; - diff --git a/fs/isofs/export.c b/fs/isofs/export.c index 78f80c1a5c54..4f7fa1d508a1 100644 --- a/fs/isofs/export.c +++ b/fs/isofs/export.c @@ -83,13 +83,21 @@ static struct dentry *isofs_export_get_parent(struct dentry *child) /* This is the "." entry. */ de = (struct iso_directory_record*)bh->b_data; + if (!isofs_dir_record_valid(de, 0, child_inode->i_sb->s_blocksize) || + isonum_711(de->name_len) != 1 || de->name[0] != 0) { + printk(KERN_ERR "isofs: Unable to find the \".\" directory for NFS.\n"); + rv = ERR_PTR(-EACCES); + goto out; + } /* The ".." entry is always the second entry. */ parent_offset = (unsigned long)isonum_711(de->length); de = (struct iso_directory_record*)(bh->b_data + parent_offset); /* Verify it is in fact the ".." entry. */ - if ((isonum_711(de->name_len) != 1) || (de->name[0] != 1)) { + if (!isofs_dir_record_valid(de, parent_offset, + child_inode->i_sb->s_blocksize) || + isonum_711(de->name_len) != 1 || de->name[0] != 1) { printk(KERN_ERR "isofs: Unable to find the \"..\" " "directory for NFS.\n"); rv = ERR_PTR(-EACCES); diff --git a/fs/isofs/isofs.h b/fs/isofs/isofs.h index 0ec8b24a42ed..dacb9cdae4fd 100644 --- a/fs/isofs/isofs.h +++ b/fs/isofs/isofs.h @@ -115,6 +115,9 @@ struct inode; /* To make gcc happy */ extern int parse_rock_ridge_inode(struct iso_directory_record *, struct inode *, int relocated); extern int get_rock_ridge_filename(struct iso_directory_record *, char *, struct inode *); extern int isofs_name_translate(struct iso_directory_record *, char *, struct inode *); +bool isofs_dir_record_valid(struct iso_directory_record *de, + unsigned long offset, + unsigned long bufsize); int get_joliet_filename(struct iso_directory_record *, unsigned char *, struct inode *); int get_acorn_filename(struct iso_directory_record *, char *, struct inode *); diff --git a/fs/isofs/namei.c b/fs/isofs/namei.c index 3ace3d6a55e7..a161b28893d6 100644 --- a/fs/isofs/namei.c +++ b/fs/isofs/namei.c @@ -10,6 +10,26 @@ #include <linux/gfp.h> #include "isofs.h" +bool isofs_dir_record_valid(struct iso_directory_record *de, + unsigned long offset, + unsigned long bufsize) +{ + unsigned int len; + unsigned int name_len; + unsigned long min_len = offsetof(struct iso_directory_record, name); + + if (offset > bufsize || bufsize - offset < min_len) + return false; + + len = isonum_711(de->length); + name_len = isonum_711(de->name_len); + if (len < min_len || name_len > len - min_len) + return false; + if (len > bufsize - offset) + return false; + return true; +} + static int isofs_cmp(struct dentry *dentry, const char *compare, int dlen) { @@ -88,16 +108,16 @@ isofs_find_entry(struct inode *dir, struct dentry *dentry, de = tmpde; } - dlen = de->name_len[0]; - dpnt = de->name; - /* Basic sanity check, whether name doesn't exceed dir entry */ - if (de_len < dlen + sizeof(struct iso_directory_record)) { + if (!isofs_dir_record_valid(de, de == tmpde ? 0 : offset_saved, + de == tmpde ? de_len : bufsize)) { printk(KERN_NOTICE "iso9660: Corrupted directory entry" " in block %lu of inode %llu\n", block, dir->i_ino); brelse(bh); return 0; } + dlen = de->name_len[0]; + dpnt = de->name; if (sbi->s_rock && ((i = get_rock_ridge_filename(de, tmpname, dir)))) { -- 2.51.0 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] isofs: validate directory records consistently 2026-07-28 7:43 ` [PATCH v2] isofs: validate directory records consistently Yichong Chen @ 2026-07-28 16:05 ` Jan Kara 0 siblings, 0 replies; 5+ messages in thread From: Jan Kara @ 2026-07-28 16:05 UTC (permalink / raw) To: Yichong Chen; +Cc: jack, linux-fsdevel, linux-kernel On Tue 28-07-26 15:43:49, Yichong Chen wrote: > isofs_export_get_parent() assumes that the first two directory records > are valid "." and ".." entries. A malformed image can provide an > invalid length for the first entry, causing the computed ".." offset to > point outside the received block. > > Add a shared directory record validator and use it in NFS get_parent, > readdir and lookup. This keeps the basic directory record length checks > consistent across all directory users before they consume the name field > or use one record length to find the next entry. > > Signed-off-by: Yichong Chen <chenyichong@uniontech.com> Thanks! I've merged this patch to my tree. Honza > --- > v2: > - Move the directory record validator to namei.c. > - Share the validator with NFS get_parent, readdir and lookup. > - Replace duplicate directory record length checks. > > fs/isofs/dir.c | 7 ++----- > fs/isofs/export.c | 10 +++++++++- > fs/isofs/isofs.h | 3 +++ > fs/isofs/namei.c | 28 ++++++++++++++++++++++++---- > 4 files changed, 38 insertions(+), 10 deletions(-) > > diff --git a/fs/isofs/dir.c b/fs/isofs/dir.c > index cc587cd25162..a96268c9ca41 100644 > --- a/fs/isofs/dir.c > +++ b/fs/isofs/dir.c > @@ -149,10 +149,8 @@ static int do_isofs_readdir(struct inode *inode, struct file *file, > } > de = tmpde; > } > - /* Basic sanity check, whether name doesn't exceed dir entry */ > - if (de_len < sizeof(struct iso_directory_record) || > - de_len < de->name_len[0] + > - sizeof(struct iso_directory_record)) { > + if (!isofs_dir_record_valid(de, de == tmpde ? 0 : offset_saved, > + de == tmpde ? de_len : bufsize)) { > printk(KERN_NOTICE "iso9660: Corrupted directory entry" > " in block %lu of inode %llu\n", block, > inode->i_ino); > @@ -300,4 +298,3 @@ const struct inode_operations isofs_dir_inode_operations = > .fileattr_get = isofs_fileattr_get, > }; > > - > diff --git a/fs/isofs/export.c b/fs/isofs/export.c > index 78f80c1a5c54..4f7fa1d508a1 100644 > --- a/fs/isofs/export.c > +++ b/fs/isofs/export.c > @@ -83,13 +83,21 @@ static struct dentry *isofs_export_get_parent(struct dentry *child) > > /* This is the "." entry. */ > de = (struct iso_directory_record*)bh->b_data; > + if (!isofs_dir_record_valid(de, 0, child_inode->i_sb->s_blocksize) || > + isonum_711(de->name_len) != 1 || de->name[0] != 0) { > + printk(KERN_ERR "isofs: Unable to find the \".\" directory for NFS.\n"); > + rv = ERR_PTR(-EACCES); > + goto out; > + } > > /* The ".." entry is always the second entry. */ > parent_offset = (unsigned long)isonum_711(de->length); > de = (struct iso_directory_record*)(bh->b_data + parent_offset); > > /* Verify it is in fact the ".." entry. */ > - if ((isonum_711(de->name_len) != 1) || (de->name[0] != 1)) { > + if (!isofs_dir_record_valid(de, parent_offset, > + child_inode->i_sb->s_blocksize) || > + isonum_711(de->name_len) != 1 || de->name[0] != 1) { > printk(KERN_ERR "isofs: Unable to find the \"..\" " > "directory for NFS.\n"); > rv = ERR_PTR(-EACCES); > diff --git a/fs/isofs/isofs.h b/fs/isofs/isofs.h > index 0ec8b24a42ed..dacb9cdae4fd 100644 > --- a/fs/isofs/isofs.h > +++ b/fs/isofs/isofs.h > @@ -115,6 +115,9 @@ struct inode; /* To make gcc happy */ > extern int parse_rock_ridge_inode(struct iso_directory_record *, struct inode *, int relocated); > extern int get_rock_ridge_filename(struct iso_directory_record *, char *, struct inode *); > extern int isofs_name_translate(struct iso_directory_record *, char *, struct inode *); > +bool isofs_dir_record_valid(struct iso_directory_record *de, > + unsigned long offset, > + unsigned long bufsize); > > int get_joliet_filename(struct iso_directory_record *, unsigned char *, struct inode *); > int get_acorn_filename(struct iso_directory_record *, char *, struct inode *); > diff --git a/fs/isofs/namei.c b/fs/isofs/namei.c > index 3ace3d6a55e7..a161b28893d6 100644 > --- a/fs/isofs/namei.c > +++ b/fs/isofs/namei.c > @@ -10,6 +10,26 @@ > #include <linux/gfp.h> > #include "isofs.h" > > +bool isofs_dir_record_valid(struct iso_directory_record *de, > + unsigned long offset, > + unsigned long bufsize) > +{ > + unsigned int len; > + unsigned int name_len; > + unsigned long min_len = offsetof(struct iso_directory_record, name); > + > + if (offset > bufsize || bufsize - offset < min_len) > + return false; > + > + len = isonum_711(de->length); > + name_len = isonum_711(de->name_len); > + if (len < min_len || name_len > len - min_len) > + return false; > + if (len > bufsize - offset) > + return false; > + return true; > +} > + > static int > isofs_cmp(struct dentry *dentry, const char *compare, int dlen) > { > @@ -88,16 +108,16 @@ isofs_find_entry(struct inode *dir, struct dentry *dentry, > de = tmpde; > } > > - dlen = de->name_len[0]; > - dpnt = de->name; > - /* Basic sanity check, whether name doesn't exceed dir entry */ > - if (de_len < dlen + sizeof(struct iso_directory_record)) { > + if (!isofs_dir_record_valid(de, de == tmpde ? 0 : offset_saved, > + de == tmpde ? de_len : bufsize)) { > printk(KERN_NOTICE "iso9660: Corrupted directory entry" > " in block %lu of inode %llu\n", block, > dir->i_ino); > brelse(bh); > return 0; > } > + dlen = de->name_len[0]; > + dpnt = de->name; > > if (sbi->s_rock && > ((i = get_rock_ridge_filename(de, tmpname, dir)))) { > -- > 2.51.0 -- Jan Kara <jack@suse.com> SUSE Labs, CR ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-07-28 16:05 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-07-21 10:13 [PATCH] isofs: validate directory records in NFS get_parent Yichong Chen 2026-07-27 15:22 ` Jan Kara 2026-07-28 1:42 ` Yichong Chen 2026-07-28 7:43 ` [PATCH v2] isofs: validate directory records consistently Yichong Chen 2026-07-28 16:05 ` Jan Kara
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®