linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Colin Ian King <colin.king@canonical.com>
To: Yang Shi <yang.shi@linux.alibaba.com>,
	Vlastimil Babka <vbabka@suse.cz>,
	"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Stephen Rothwell <sfr@canb.auug.org.au>
Cc: linux-mm@kvack.org,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: re: mm: brk: downgrade mmap_sem to read when shrinking
Date: Thu, 4 Oct 2018 19:08:13 +0100	[thread overview]
Message-ID: <3fe71059-557b-4bab-dc88-4d0c5cfd1845@canonical.com> (raw)

Hi,

Static analysis has found a couple of issues as follows:

commit 551f205aff9198e17add1264dd781771d1a2bd9d
Author: Yang Shi <yang.shi@linux.alibaba.com>
Date:   Thu Oct 4 07:43:18 2018 +1000

    mm: brk: downgrade mmap_sem to read when shrinking

Static analysis with CoverityScan has detected an issue in mm/mmap.c,
function do_brk_flags():

                retval = __do_munmap(mm, newbrk, oldbrk-newbrk, &uf, true);
                if (retval < 0) {
                        mm->brk = origbrk;
                        goto out;
                } else if (retval == 1)
                        downgraded = true;

retval is unsigned long, so the retval < 0 check is always false, which
looks bogus to me.

Also same kind of issue with:

commit e66477708ec2a764d3add92ca59134e3812da0bb
Author: Yang Shi <yang.shi@linux.alibaba.com>
Date:   Thu Oct 4 07:43:18 2018 +1000

    mm: mremap: downgrade mmap_sem to read when shrinking

                ret = __do_munmap(mm, addr+new_len, old_len - new_len,
                                  &uf_unmap, true);
                if (ret < 0 && old_len != new_len)
                        goto out;
                /* Returning 1 indicates mmap_sem is downgraded to read. */
                else if (ret == 1)
                        downgraded = true;

again, ret is unsigned long, so the comparison with ret < 0 is always false.

Detected by CoverityScan, CID#1473794, CID#1473791 "Unsigned compared
against 0".

Colin

                 reply	other threads:[~2018-10-04 18:08 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

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=3fe71059-557b-4bab-dc88-4d0c5cfd1845@canonical.com \
    --to=colin.king@canonical.com \
    --cc=akpm@linux-foundation.org \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=sfr@canb.auug.org.au \
    --cc=vbabka@suse.cz \
    --cc=yang.shi@linux.alibaba.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