kevin/linux
1
Fork 0
Code

qnx4: avoid stringop-overread errors

Browse Source 
The qnx4 directory entries are 64-byte blocks that have different
contents depending on the a status byte that is in the last byte of the
block.

In particular, a directory entry can be either a "link info" entry with
a 48-byte name and pointers to the real inode information, or an "inode
entry" with a smaller 16-byte name and the full inode information.

But the code was written to always just treat the directory name as if
it was part of that "inode entry", and just extend the name to the
longer case if the status byte said it was a link entry.

That work just fine and gives the right results, but now that gcc is
tracking data structure accesses much more, the code can trigger a
compiler error about using up to 48 bytes (the long name) in a structure
that only has that shorter name in it:

   fs/qnx4/dir.c: In function ‘qnx4_readdir’:
   fs/qnx4/dir.c:51:32: error: ‘strnlen’ specified bound 48 exceeds source size 16 [-Werror=stringop-overread]
      51 |                         size = strnlen(de->di_fname, size);
         |                                ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   In file included from fs/qnx4/qnx4.h:3,
                    from fs/qnx4/dir.c:16:
   include/uapi/linux/qnx4_fs.h:45:25: note: source object declared here
      45 |         char            di_fname[QNX4_SHORT_NAME_MAX];
         |                         ^~~~~~~~

which is because the source code doesn't really make this whole "one of
two different types" explicit.

Fix this by introducing a very explicit union of the two types, and
basically explaining to the compiler what is really going on.

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
This commit is contained in:
Linus Torvalds 2021-09-15 13:56:37 -07:00
parent fc7c028dcd
commit b7213ffa0e
1 changed files with 34 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

51
fs/qnx4/dir.c
View File

@ -15,13 +15,27 @@
#include <linux/buffer_head.h>
#include "qnx4.h"
/*
* A qnx4 directory entry is an inode entry or link info
* depending on the status field in the last byte. The
* first byte is where the name start either way, and a
* zero means it's empty.
*/
union qnx4_directory_entry {
struct {
char de_name;
char de_pad[62];
char de_status;
};
struct qnx4_inode_entry inode;
struct qnx4_link_info link;
};
static int qnx4_readdir(struct file *file, struct dir_context *ctx)
{
struct inode *inode = file_inode(file);
unsigned int offset;
struct buffer_head *bh;
struct qnx4_inode_entry *de;
struct qnx4_link_info *le;
unsigned long blknum;
int ix, ino;
int size;
@ -38,27 +52,30 @@ static int qnx4_readdir(struct file *file, struct dir_context *ctx)
}
ix = (ctx->pos >> QNX4_DIR_ENTRY_SIZE_BITS) % QNX4_INODES_PER_BLOCK;
for (; ix < QNX4_INODES_PER_BLOCK; ix++, ctx->pos += QNX4_DIR_ENTRY_SIZE) {
union qnx4_directory_entry *de;
const char *name;
offset = ix * QNX4_DIR_ENTRY_SIZE;
de = (struct qnx4_inode_entry *) (bh->b_data + offset);
if (!de->di_fname[0])
de = (union qnx4_directory_entry *) (bh->b_data + offset);
if (!de->de_name)
continue;
if (!(de->di_status & (QNX4_FILE_USED|QNX4_FILE_LINK)))
if (!(de->de_status & (QNX4_FILE_USED|QNX4_FILE_LINK)))
continue;
if (!(de->di_status & QNX4_FILE_LINK))
size = QNX4_SHORT_NAME_MAX;
else
size = QNX4_NAME_MAX;
size = strnlen(de->di_fname, size);
QNX4DEBUG((KERN_INFO "qnx4_readdir:%.*s\n", size, de->di_fname));
if (!(de->di_status & QNX4_FILE_LINK))
if (!(de->de_status & QNX4_FILE_LINK)) {
size = sizeof(de->inode.di_fname);
name = de->inode.di_fname;
ino = blknum * QNX4_INODES_PER_BLOCK + ix - 1;
else {
le = (struct qnx4_link_info*)de;
ino = ( le32_to_cpu(le->dl_inode_blk) - 1 ) *
} else {
size = sizeof(de->link.dl_fname);
name = de->link.dl_fname;
ino = ( le32_to_cpu(de->link.dl_inode_blk) - 1 ) *
QNX4_INODES_PER_BLOCK +
le->dl_inode_ndx;
de->link.dl_inode_ndx;
}
if (!dir_emit(ctx, de->di_fname, size, ino, DT_UNKNOWN)) {
size = strnlen(name, size);
QNX4DEBUG((KERN_INFO "qnx4_readdir:%.*s\n", size, name));
if (!dir_emit(ctx, name, size, ino, DT_UNKNOWN)) {
brelse(bh);
return 0;
}