From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753547AbbFDNpc (ORCPT ); Thu, 4 Jun 2015 09:45:32 -0400 Received: from mout.gmx.net ([212.227.15.15]:51988 "EHLO mout.gmx.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753410AbbFDNp0 (ORCPT ); Thu, 4 Jun 2015 09:45:26 -0400 Message-ID: <55705670.9000808@gmx.de> Date: Thu, 04 Jun 2015 15:45:20 +0200 From: Helge Deller User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Linus Torvalds CC: Al Viro , Linux Kernel Mailing List Subject: Re: [PATCH] compat: fix possible out-of-bound accesses in compat_get_bitmap() and compat_put_bitmap() References: <20150601184437.GA2534@ls3530.box> In-Reply-To: Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-Provags-ID: V03:K0:9TWfARm+uTIO3/1ZaGhbma/k6SZc+RkCe8v9tN0/7UJuRPC/Muj JtKlcduq2epx9Rjg9GsXTeur0r7FoVJ21ZTtGEQKNNnCudQ85OB3CO1VEXgW2UcvjWn/hU7 Sb5EbOWyKbd5zSkusoGw6HT5UU75ku86y1epfQy/IyuI8BpYnjZE95pmewDFTV4GDk5kEQQ oIKQtoCgnKwn3gJPyljKQ== X-UI-Out-Filterresults: notjunk:1;V01:K0:NkY9P27756E=:otcqdgOIb70y/UEUfBHuKf ypmMX9avuvYTv7a8hpRui4rzDmcAiqC060GnzxhtGj0yV0loQGAP0BxGtpFzI2sI4qceM0Rtt J8+irErTIYK7pErbLF7sxPqV8/T9x/v1vfqECgZDKDCgvKhfXNG8Q6n+VCO3cQWILw3lzApVY h8RCnQandRfxu6CiU7lKHy9AxkB4szE/VXWBldbF00ipK0VfqfJ212HNPj85PrTjY7zaFQ1P5 3IyYDrbtUhhd28GURgMMXxGmrR6F5xYVcxklq1nVpMMXiOJ9rob17ixCZemEICoo2UvMfSQ3q deFQsmqQRhk5Imxx0/wtqCiseCU1CktSbmES0CuX+QQpbcbClkE5lIf485bNGO8b8D+2MmVLb F1osKE7e+2ED2ndUwWkMxJBkJLHsItaoqnrEhL+eHWCDsF+ZIZVuL6iTtaeKZkghvha33PPgJ SXYNXoe20uv/Idm+wzMzv0O6U7DYbuQnxwqo2g+KfOMChnpWGK7t4Sa3Aspobad4yeMOjJqtE iFt3qpjtilO/filHw30GIXjqcYCRUDSe4ewGFxVYS0JtZuHZQ+k4xKTX2ix5kq0XrefbyskYN lQac1EErJk46iAsexaveFBIwOTvnDYL5+LC/UBFEEzDjAYBYMzSvJPegWiu94gWmPuVbQwkmK gC4IbZ6YwrO7Q5Q6XM+npfdzdNzshGxzTjIESukFfwjAUK93aLiuEByucYepdW4pPpEk= Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Linus, On 02.06.2015 03:49, Linus Torvalds wrote: > On Mon, Jun 1, 2015 at 11:44 AM, Helge Deller wrote: >> >> Since nr_compat_longs gets unconditionally decremented in each loop, it's type >> needs to be signed instead of unsigned to avoid possibly accessing userspace >> memory behind the bitmap which shouldn't be accessed. > > I'd actually prefer to instead just make the decrement conditional, > since that would seem to be the more obvious code. Make the logic be > "iff I have more to go, do the access, and then decrement the counter" That's fine for me. I just wanted to keep the patch small, but your proposal makes the code of course more human readable. > Also, compat_put_bitmap() has the exact same code, and should have the same fix. > > Finally, I don't think this is an *actual* bug, just bad and stupid > code. The thing is, the inner loop is only executed twice anyway, and > on that last iteration where "nr_compat_longs" could go negative, the > _outer_ loop will break out too. So there is no actual way we can > enter the thing with nr_compat_longs <= 1 to begin with. > > So I don't think the code ever really actually overflows. Yes, it probably doesn't overflows. It's not that easy to follow all code paths (and take into account that the bitmap sizes might be different), but the code in general looks clean. > I do agree > that the code looks bad, so I think a patch like the attached would be > a good idea. Not necessarily marked for stable, unless you can point > out why I'm wrong about the edge condition. No, your code proposal is fine. Do you want me to send it again cleaned up, or will you just take yours? Thanks! Helge