linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Earl Chew <earl_chew@agilent.com>
To: Johannes Weiner <hannes@cmpxchg.org>
Cc: linux-kernel@vger.kernel.org
Subject: Re: Arithmetic overflow in may_expand_vm()
Date: Mon, 19 Oct 2009 07:43:01 -0700	[thread overview]
Message-ID: <4ADC7AF5.2020707@agilent.com> (raw)
In-Reply-To: <20091019075350.GA1769@cmpxchg.org>

Johannes Weiner wrote:
>> If npages is stupendously large, the failure predicate may
>> return a false negative due to (cur + npages) overflowing and
>> wrapping.
> 
> Can this really happen?
> 
> npages always originates in a value of byte granularity, giving a
> theoretical maximum of ~0UL >> PAGE_SHIFT (checking for more than the
> number of addressable bytes just makes no sense).

I think you're saying that there are no (external facing)
interfaces that ask for pages -- they always ask for octets.

You may well be right. I don't know the kernel code base well
enough to say for sure one way or another.

> And mm->total_vm is always PAGE_SIZE times smaller than total user
> address space (which in turn is always less than ~0UL).
> 
> So I can not see this overflow being possible with PAGE_SHIFT > 0.

A very reasonable argument to be sure.

I can think of two counter-arguments:

a. The fewer assumptions made by may_expand_vm() (or any other
    function for that matter) about its callers, the more robust
    the function, and the more resilient the system.

    I think it would be good practice for may_expand_vm() to
    do the right thing for all possible input values. Especially
    in this case where the cost of doing the right thing is either
    very small or zero.

b. There are other examples in the code base that use the more
    robust approach. For instance see kernel/filemap.c:

         unsigned long limit =
            current->signal->rlim[RLIMIT_FSIZE].rlim_cur;

            ... snip ...

                 if (limit != RLIM_INFINITY) {
                         if (*pos >= limit) {
                                 send_sig(SIGXFSZ, current, 0);
                                 return -EFBIG;
                         }
                         if (*count > limit - (typeof(limit))*pos) {
                                 *count = limit - (typeof(limit))*pos;
                         }
                 }


Earl




  reply	other threads:[~2009-10-19 14:43 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-10-15 17:24 Arithmetic overflow in may_expand_vm() Earl Chew
2009-10-19  7:53 ` Johannes Weiner
2009-10-19 14:43   ` Earl Chew [this message]
2009-10-19 23:40     ` Johannes Weiner

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=4ADC7AF5.2020707@agilent.com \
    --to=earl_chew@agilent.com \
    --cc=hannes@cmpxchg.org \
    --cc=linux-kernel@vger.kernel.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).