public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Kees Cook <keescook@chromium.org>
Cc: Chen Gang <gang.chen@asianux.com>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	Serge Hallyn <serge.hallyn@canonical.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] fs/befs/linuxvfs.c: need signed cast for variable 'block'
Date: Thu, 31 Oct 2013 19:06:08 +0000	[thread overview]
Message-ID: <20131031190608.GH13318@ZenIV.linux.org.uk> (raw)
In-Reply-To: <CAGXu5jLMU0+VkJ6N3NTKbEtL+9FwF2gMOTU86-GH6SSXQ0rc0w@mail.gmail.com>

On Thu, Oct 31, 2013 at 09:53:59AM -0700, Kees Cook wrote:

> If block (type sector_t) is unsigned, we shouldn't cast it signed.
> This entire code path should be removed. What is BEFS's expected
> maximum block size? (Looks like even befs_blocknr_t is u64, so nothing
> seems trivially in danger of wrapping.) I would also note that all the
> format strings are wrong too (%ld instead of %lu).

FWIW, this
        res = befs_fblock2brun(sb, ds, block, &run);
        if (res != BEFS_OK) {
                befs_error(sb,
                           "<--- befs_get_block() for inode %lu, block "
                           "%ld ERROR", inode->i_ino, block);
                return -EFBIG;
        }
also looks wrong - ioctl(..., FIBMAP, ...) shouldn't be able to spew
printks on a valid fs and hitting it with block number greater than
file length will, AFAICS, trigger that.

I agree that this code needs fixing, but just making gcc STFU about the
comparison would only serve to hide the problem.  Anybody familiar with
befs or willing to learn it?

  reply	other threads:[~2013-10-31 19:06 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-31  2:52 [PATCH] fs/befs/linuxvfs.c: need signed cast for variable 'block' Chen Gang
2013-10-31 16:53 ` Kees Cook
2013-10-31 19:06   ` Al Viro [this message]
2013-10-31 19:08     ` Kees Cook
2013-10-31 20:45       ` Greg KH
2013-11-01  2:41         ` Chen Gang
2013-11-02 13:46           ` Chen Gang
2013-11-02 15:44             ` Greg KH
2013-11-02 16:27               ` Al Viro
2013-11-03 12:41                 ` Chen Gang

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=20131031190608.GH13318@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=ebiederm@xmission.com \
    --cc=gang.chen@asianux.com \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=serge.hallyn@canonical.com \
    /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