From: Anders Larsen <al@alarsen.net>
To: Linus Torvalds <torvalds@linux-foundation.org>,
Arnd Bergmann <arnd@kernel.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] [RFC v2] qnx: avoid -Wstringop-overread warning, again
Date: Tue, 21 Sep 2021 17:15:33 +0200 [thread overview]
Message-ID: <2955101.xlVK0Xs8nM@alarsen.net> (raw)
In-Reply-To: <CAK8P3a03VTsdALMORVSWvAY9J8dS=wQjvhf=M0hXGqLLxDYHsQ@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1711 bytes --]
On Tuesday, 2021-09-21 10:18 Arnd Bergmann wrote:
> On Mon, Sep 20, 2021 at 7:26 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> > It sounds like we can avoid the gcc bug if we just always use
> > "de->de_name[]". Then we don't need to depend on magical behavior
> > about one particular gcc version and a strange empty array in front of
> > it.
> >
> > IOW, something like the attached simpler thing that just does that
> > "always use de_name[]" and has a comment about why we don't do the
> > natural thing
well, the code in question actually does not use anything from struct
qnx4_inode_entry except di_fname and di_status;
they are available at the same offsets in struct qnx4_link_info as well, so
wouldn't it be even simpler to just always use the fields of the latter
structure?
Like in the attached patch which replaces b7213ffa0e58?
($me feeling bad for reverting Linus' patch!)
That way, the compiler should never see any access to the (shorter)
qnx4_inode_entry.di_fname
BTW, in the process I noticed that fs/qnx4/namei.c was missed by 663f4deca76
back in 2013 and so is still calling strlen() on untrusted data; the second
part of the patch takes care of that.
> > Also, just what version of gcc is the broken one? You say "gcc-11",
> > but I certainly don't see it with _my_ version of gcc-11, so can we
> > (just for that comment) document more precisely what version you have
> > (or possibly what config you use to trigger it).
>
> I'm using the gcc-11.1.0 that I uploaded to
> https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/11.1.0/
I don't have that compiler version, so obviously I couldn't test if the patch
solves the problem.
Cheers
Anders
[-- Attachment #2: patch.diff --]
[-- Type: text/x-patch, Size: 4908 bytes --]
fs/qnx4/dir.c | 18 ++++++++----------
fs/qnx4/namei.c | 34 +++++++++++++++-------------------
2 files changed, 23 insertions(+), 29 deletions(-)
diff --git a/fs/qnx4/dir.c b/fs/qnx4/dir.c
index a6ee23aadd28..45b0262c6fac 100644
--- a/fs/qnx4/dir.c
+++ b/fs/qnx4/dir.c
@@ -20,7 +20,6 @@ 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;
@@ -39,26 +38,25 @@ 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) {
offset = ix * QNX4_DIR_ENTRY_SIZE;
- de = (struct qnx4_inode_entry *) (bh->b_data + offset);
- if (!de->di_fname[0])
+ le = (struct qnx4_link_info *) (bh->b_data + offset);
+ if (!le->dl_fname[0])
continue;
- if (!(de->di_status & (QNX4_FILE_USED|QNX4_FILE_LINK)))
+ if (!(le->dl_status & (QNX4_FILE_USED|QNX4_FILE_LINK)))
continue;
- if (!(de->di_status & QNX4_FILE_LINK))
+ if (!(le->dl_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))
+ size = strnlen(le->dl_fname, size);
+ QNX4DEBUG((KERN_INFO "qnx4_readdir:%.*s\n", size, le->dl_fname));
+ if (!(le->dl_status & QNX4_FILE_LINK))
ino = blknum * QNX4_INODES_PER_BLOCK + ix - 1;
else {
- le = (struct qnx4_link_info*)de;
ino = ( le32_to_cpu(le->dl_inode_blk) - 1 ) *
QNX4_INODES_PER_BLOCK +
le->dl_inode_ndx;
}
- if (!dir_emit(ctx, de->di_fname, size, ino, DT_UNKNOWN)) {
+ if (!dir_emit(ctx, le->dl_fname, size, ino, DT_UNKNOWN)) {
brelse(bh);
return 0;
}
diff --git a/fs/qnx4/namei.c b/fs/qnx4/namei.c
index 8d72221735d7..75ff330ce5e0 100644
--- a/fs/qnx4/namei.c
+++ b/fs/qnx4/namei.c
@@ -26,28 +26,26 @@
static int qnx4_match(int len, const char *name,
struct buffer_head *bh, unsigned long *offset)
{
- struct qnx4_inode_entry *de;
- int namelen, thislen;
+ struct qnx4_link_info *le;
+ int namelen;
if (bh == NULL) {
printk(KERN_WARNING "qnx4: matching unassigned buffer !\n");
return 0;
}
- de = (struct qnx4_inode_entry *) (bh->b_data + *offset);
+ le = (struct qnx4_link_info *) (bh->b_data + *offset);
*offset += QNX4_DIR_ENTRY_SIZE;
- if ((de->di_status & QNX4_FILE_LINK) != 0) {
+ if ((le->dl_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) {
+ namelen = strnlen( le->dl_fname, namelen );
+ if (len != namelen) {
return 0;
}
- if (strncmp(name, de->di_fname, len) == 0) {
- if ((de->di_status & (QNX4_FILE_USED|QNX4_FILE_LINK)) != 0) {
+ if (strncmp(name, le->dl_fname, len) == 0) {
+ if ((le->dl_status & (QNX4_FILE_USED|QNX4_FILE_LINK)) != 0) {
return 1;
}
}
@@ -55,7 +53,7 @@ static int qnx4_match(int len, const char *name,
}
static struct buffer_head *qnx4_find_entry(int len, struct inode *dir,
- const char *name, struct qnx4_inode_entry **res_dir, int *ino)
+ const char *name, struct qnx4_link_info **res_dir, int *ino)
{
unsigned long block, offset, blkofs;
struct buffer_head *bh;
@@ -73,7 +71,7 @@ static struct buffer_head *qnx4_find_entry(int len, struct inode *dir,
continue;
}
}
- *res_dir = (struct qnx4_inode_entry *) (bh->b_data + offset);
+ *res_dir = (struct qnx4_link_info *) (bh->b_data + offset);
if (qnx4_match(len, name, bh, &offset)) {
*ino = block * QNX4_INODES_PER_BLOCK +
(offset / QNX4_DIR_ENTRY_SIZE) - 1;
@@ -95,21 +93,19 @@ static struct buffer_head *qnx4_find_entry(int len, struct inode *dir,
struct dentry * qnx4_lookup(struct inode *dir, struct dentry *dentry, unsigned int flags)
{
int ino;
- struct qnx4_inode_entry *de;
- struct qnx4_link_info *lnk;
+ struct qnx4_link_info *le;
struct buffer_head *bh;
const char *name = dentry->d_name.name;
int len = dentry->d_name.len;
struct inode *foundinode = NULL;
- if (!(bh = qnx4_find_entry(len, dir, name, &de, &ino)))
+ if (!(bh = qnx4_find_entry(len, dir, name, &le, &ino)))
goto out;
/* The entry is linked, let's get the real info */
- if ((de->di_status & QNX4_FILE_LINK) == QNX4_FILE_LINK) {
- lnk = (struct qnx4_link_info *) de;
- ino = (le32_to_cpu(lnk->dl_inode_blk) - 1) *
+ if ((le->dl_status & QNX4_FILE_LINK) == QNX4_FILE_LINK) {
+ ino = (le32_to_cpu(le->dl_inode_blk) - 1) *
QNX4_INODES_PER_BLOCK +
- lnk->dl_inode_ndx;
+ le->dl_inode_ndx;
}
brelse(bh);
next prev parent reply other threads:[~2021-09-21 15:15 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-20 12:12 [PATCH] [RFC v2] qnx: avoid -Wstringop-overread warning, again Arnd Bergmann
2021-09-20 17:26 ` Linus Torvalds
2021-09-21 8:18 ` Arnd Bergmann
2021-09-21 15:15 ` Anders Larsen [this message]
2021-09-21 15:40 ` Linus Torvalds
2021-09-21 15:49 ` Anders Larsen
2021-09-21 15:51 ` Linus Torvalds
2021-09-21 16:56 ` Arnd Bergmann
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2955101.xlVK0Xs8nM@alarsen.net \
--to=al@alarsen.net \
--cc=arnd@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).