Re: [bug report] mm: brk: downgrade mmap_sem to read when shrinking

[Yang Shi <yang.shi@linux.alibaba.com>]

Hi Dan,


Thanks for reporting this bug. Colin Ian King reported the same issue a 
couple of days ago, please see 
https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1780112.html.


And, the fixes have been merged into Andrew's -mm tree.


Regards,

Yang



On 10/10/18 3:44 AM, Dan Carpenter wrote:
> Hello Yang Shi,
>
> The patch ca761a3ea456: "mm: brk: downgrade mmap_sem to read when
> shrinking" from Oct 4, 2018, leads to the following static checker
> warning:
>
> 	mm/mmap.c:252 __do_sys_brk()
> 	warn: unsigned 'retval' is never less than zero.
>
> mm/mmap.c
>     223          /*
>     224           * Check against rlimit here. If this check is done later after the test
>     225           * of oldbrk with newbrk then it can escape the test and let the data
>     226           * segment grow beyond its set limit the in case where the limit is
>     227           * not page aligned -Ram Gupta
>     228           */
>     229          if (check_data_rlimit(rlimit(RLIMIT_DATA), brk, mm->start_brk,
>     230                                mm->end_data, mm->start_data))
>     231                  goto out;
>     232
>     233          newbrk = PAGE_ALIGN(brk);
>     234          oldbrk = PAGE_ALIGN(mm->brk);
>     235          if (oldbrk == newbrk) {
>     236                  mm->brk = brk;
>     237                  goto success;
>     238          }
>     239
>     240          /*
>     241           * Always allow shrinking brk.
>     242           * __do_munmap() may downgrade mmap_sem to read.
>     243           */
>     244          if (brk <= mm->brk) {
>     245                  /*
>     246                   * mm->brk must to be protected by write mmap_sem so update it
>     247                   * before downgrading mmap_sem. When __do_munmap() fails,
>     248                   * mm->brk will be restored from origbrk.
>     249                   */
>     250                  mm->brk = brk;
>     251                  retval = __do_munmap(mm, newbrk, oldbrk-newbrk, &uf, true);
>     252                  if (retval < 0) {
>                              ^^^^^^^^^^
> Impossible.
>
>     253                          mm->brk = origbrk;
>     254                          goto out;
>     255                  } else if (retval == 1)
>     256                          downgraded = true;
>     257                  goto success;
>     258          }
>     259
>
> See also:
> mm/mremap.c:571 __do_sys_mremap() warn: unsigned 'ret' is never less than zero.
>
> regards,
> dan carpenter