Hi,
This attempts to fix the issue Ronald Monthero found[1]. Avoids using a
too-short struct buffer when reading the string, by using the existing
struct union.
-Kees
[1] https://lore.kernel.org/lkml/[email protected]/
v2:
- Use BUILD_BUG_ON() instead of _Static_assert()
v1: https://lore.kernel.org/all/[email protected]/
Kees Cook (2):
qnx4: Extract dir entry filename processing into helper
qnx4: Use get_directory_fname() in qnx4_match()
fs/qnx4/dir.c | 52 ++++++------------------------------------
fs/qnx4/namei.c | 29 +++++++++---------------
fs/qnx4/qnx4.h | 60 +++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 78 insertions(+), 63 deletions(-)
--
2.34.1
Use the new common directory entry name accessor helper to avoid
confusing the compiler about over-running the file name buffer. Avoids
false positive buffer overflow warning:
[ 4849.636861] detected buffer overflow in strlen
[ 4849.636897] ------------[ cut here ]------------
[ 4849.636902] kernel BUG at lib/string.c:1165!
...
[ 4849.637047] Call Trace:
...
[ 4849.637251] qnx4_find_entry.cold+0xc/0x18 [qnx4]
[ 4849.637264] qnx4_lookup+0x3c/0xa0 [qnx4]
Cc: Anders Larsen <[email protected]>
Reported-by: Ronald Monthero <[email protected]>
Closes: https://lore.kernel.org/lkml/[email protected]/
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Kees Cook <[email protected]>
---
fs/qnx4/namei.c | 29 +++++++++++------------------
1 file changed, 11 insertions(+), 18 deletions(-)
diff --git a/fs/qnx4/namei.c b/fs/qnx4/namei.c
index 8d72221735d7..bb8db6550ca5 100644
--- a/fs/qnx4/namei.c
+++ b/fs/qnx4/namei.c
@@ -26,31 +26,24 @@
static int qnx4_match(int len, const char *name,
struct buffer_head *bh, unsigned long *offset)
{
- struct qnx4_inode_entry *de;
- int namelen, thislen;
+ union qnx4_directory_entry *de;
+ const char *fname;
+ int fnamelen;
if (bh == NULL) {
printk(KERN_WARNING "qnx4: matching unassigned buffer !\n");
return 0;
}
- de = (struct qnx4_inode_entry *) (bh->b_data + *offset);
+ de = (union qnx4_directory_entry *) (bh->b_data + *offset);
*offset += QNX4_DIR_ENTRY_SIZE;
- if ((de->di_status & QNX4_FILE_LINK) != 0) {
- namelen = QNX4_NAME_MAX;
- } else {
- namelen = QNX4_SHORT_NAME_MAX;
- }
- thislen = strlen( de->di_fname );
- if ( thislen > namelen )
- thislen = namelen;
- if (len != thislen) {
+
+ fname = get_entry_fname(de, &fnamelen);
+ if (!fname || len != fnamelen)
return 0;
- }
- if (strncmp(name, de->di_fname, len) == 0) {
- if ((de->di_status & (QNX4_FILE_USED|QNX4_FILE_LINK)) != 0) {
- return 1;
- }
- }
+
+ if (strncmp(name, fname, len) == 0)
+ return 1;
+
return 0;
}
--
2.34.1
Cheers Kees,
BR,
ronald
On Fri, Dec 1, 2023 at 6:51 AM Kees Cook <[email protected]> wrote:
>
> Hi,
>
> This attempts to fix the issue Ronald Monthero found[1]. Avoids using a
> too-short struct buffer when reading the string, by using the existing
> struct union.
>
> -Kees
>
> [1] https://lore.kernel.org/lkml/[email protected]/
>
> v2:
> - Use BUILD_BUG_ON() instead of _Static_assert()
> v1: https://lore.kernel.org/all/[email protected]/
>
> Kees Cook (2):
> qnx4: Extract dir entry filename processing into helper
> qnx4: Use get_directory_fname() in qnx4_match()
>
> fs/qnx4/dir.c | 52 ++++++------------------------------------
> fs/qnx4/namei.c | 29 +++++++++---------------
> fs/qnx4/qnx4.h | 60 +++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 78 insertions(+), 63 deletions(-)
>
> --
> 2.34.1
>
On Tue, Dec 05, 2023 at 01:46:27AM +1000, Ronald Monthero wrote:
> Cheers Kees,
> BR,
> ronald
Is this a "Tested-by"? :)
-Kees
>
>
> On Fri, Dec 1, 2023 at 6:51 AM Kees Cook <[email protected]> wrote:
> >
> > Hi,
> >
> > This attempts to fix the issue Ronald Monthero found[1]. Avoids using a
> > too-short struct buffer when reading the string, by using the existing
> > struct union.
> >
> > -Kees
> >
> > [1] https://lore.kernel.org/lkml/[email protected]/
> >
> > v2:
> > - Use BUILD_BUG_ON() instead of _Static_assert()
> > v1: https://lore.kernel.org/all/[email protected]/
> >
> > Kees Cook (2):
> > qnx4: Extract dir entry filename processing into helper
> > qnx4: Use get_directory_fname() in qnx4_match()
> >
> > fs/qnx4/dir.c | 52 ++++++------------------------------------
> > fs/qnx4/namei.c | 29 +++++++++---------------
> > fs/qnx4/qnx4.h | 60 +++++++++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 78 insertions(+), 63 deletions(-)
> >
> > --
> > 2.34.1
> >
--
Kees Cook
On Thu, 30 Nov 2023 12:51:17 -0800, Kees Cook wrote:
> This attempts to fix the issue Ronald Monthero found[1]. Avoids using a
> too-short struct buffer when reading the string, by using the existing
> struct union.
>
> -Kees
>
> [1] https://lore.kernel.org/lkml/[email protected]/
>
> [...]
I'll put these in -next since there's been no more discussion on it.
Applied to for-next/hardening, thanks!
[1/2] qnx4: Extract dir entry filename processing into helper
https://git.kernel.org/kees/c/49a85c02a189
[2/2] qnx4: Use get_directory_fname() in qnx4_match()
https://git.kernel.org/kees/c/0a0fb20f5e08
Take care,
--
Kees Cook
Hi Kees,
On 2023-12-12 22:19 Kees Cook wrote:
> On Thu, 30 Nov 2023 12:51:17 -0800, Kees Cook wrote:
> > This attempts to fix the issue Ronald Monthero found[1]. Avoids using a
> > too-short struct buffer when reading the string, by using the existing
> > struct union.
> >
> > -Kees
> >
> > [1]
> > https://lore.kernel.org/lkml/20231112095353.579855-1-debug.penguin32@gmai
> > l.com/
> >
> > [...]
>
> I'll put these in -next since there's been no more discussion on it.
>
> Applied to for-next/hardening, thanks!
thanks for taking care of this (and apologies for me being unresponsive)
If it's not too late, feel free to add
Acked-by: Anders Larsen <[email protected]>
Cheers
Anders
On Wed, Dec 13, 2023 at 05:43:08PM +0100, Anders Larsen wrote:
> Hi Kees,
>
> On 2023-12-12 22:19 Kees Cook wrote:
> > On Thu, 30 Nov 2023 12:51:17 -0800, Kees Cook wrote:
> > > This attempts to fix the issue Ronald Monthero found[1]. Avoids using a
> > > too-short struct buffer when reading the string, by using the existing
> > > struct union.
> > >
> > > -Kees
> > >
> > > [1]
> > > https://lore.kernel.org/lkml/20231112095353.579855-1-debug.penguin32@gmai
> > > l.com/
> > >
> > > [...]
> >
> > I'll put these in -next since there's been no more discussion on it.
> >
> > Applied to for-next/hardening, thanks!
>
> thanks for taking care of this (and apologies for me being unresponsive)
>
> If it's not too late, feel free to add
> Acked-by: Anders Larsen <[email protected]>
Thanks! I'll update the tags. :)
--
Kees Cook
On Tue, Dec 5, 2023 at 8:10 AM Kees Cook <[email protected]> wrote:
>
> On Tue, Dec 05, 2023 at 01:46:27AM +1000, Ronald Monthero wrote:
> > Cheers Kees,
> > BR,
> > ronald
>
> Is this a "Tested-by"? :)
Oh sorry Kees I have somehow missed this conversation.
Yes ack the tests which were earlier causing oops, now pass with the 2 patches.
BR,
ronald