From: ebiederm@xmission.com (Eric W. Biederman)
To: David Howells <dhowells@redhat.com>
Cc: Mitsuo Hayasaka <mitsuo.hayasaka.hu@hitachi.com>,
Andrew Morton <akpm@linux-foundation.org>,
James Morris <james.l.morris@oracle.com>,
linux-kernel@vger.kernel.org, yrl.pp-manager.tt@hitachi.com
Subject: Re: [PATCH] sysctl: fix improper indication of integer sysctl parameter
Date: Tue, 31 Jul 2012 07:38:58 -0700 [thread overview]
Message-ID: <874nooc7wd.fsf@xmission.com> (raw)
In-Reply-To: <5498.1343743325@warthog.procyon.org.uk> (David Howells's message of "Tue, 31 Jul 2012 15:02:05 +0100")
David Howells <dhowells@redhat.com> writes:
> Mitsuo Hayasaka <mitsuo.hayasaka.hu@hitachi.com> wrote:
>
>> When we read the sysctl parameter, they are always treated
>> as signed integer, and are casted into unsigned long type
>> in the current kernel. If we set a value equivalent to
>> (the maximum value in signed integer + 1)
>
> Wouldn't it be better to return EINVAL or EDOM?
Yes we should definitely fail the write in the case where we write an
unsigned value and we can not fit that value in an integer.
There will still remain the bug of reading the integer
where (-val == val) && val < 0. In that case we do want to an
(unsigned int) before storing it in an unsinged long.
The decription of the patch is confusing.
The problem is not the cast to unsigned long, the problem is
the implicit cast to signed long which happens before the cast
to unsigned long. I think this is a case where C's casting rules
get it wrong. If I have an explicit cast why add an implicit
cast to do sign extension. Sigh.
Mitsuo since you are looking at this do you think you could fix the
write side of the problem as well, and check to make cetain the
unsigned value we write will fit in an interger.
Eric
prev parent reply other threads:[~2012-07-31 14:39 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-07-31 13:03 [PATCH] sysctl: fix improper indication of integer sysctl parameter Mitsuo Hayasaka
2012-07-31 14:02 ` David Howells
2012-07-31 14:38 ` Eric W. Biederman [this message]
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=874nooc7wd.fsf@xmission.com \
--to=ebiederm@xmission.com \
--cc=akpm@linux-foundation.org \
--cc=dhowells@redhat.com \
--cc=james.l.morris@oracle.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mitsuo.hayasaka.hu@hitachi.com \
--cc=yrl.pp-manager.tt@hitachi.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