[CVE-2011-1593][Hardy][PATCH 1/2] next_pidmap: fix overflow condition, CVE-2011-1593

[Brad Figg]

On 05/18/2011 02:16 PM, Herton Ronaldo Krzesinski wrote:
> From: Linus Torvalds<torvalds at linux-foundation.org>
>
> CVE-2011-1593
>
> BugLink: https://bugs.launchpad.net/bugs/784727
>
> Released until now with stable versions 2.6.27.59, 2.6.32.39, 2.6.33.12,
> 2.6.35.13, 2.6.38.4
>
> next_pidmap() just quietly accepted whatever 'last' pid that was passed
> in, which is not all that safe when one of the users is /proc.
>
> Admittedly the proc code should do some sanity checking on the range
> (and that will be the next commit), but that doesn't mean that the
> helper functions should just do that pidmap pointer arithmetic without
> checking the range of its arguments.
>
> So clamp 'last' to PID_MAX_LIMIT.  The fact that we then do "last+1"
> doesn't really matter, the for-loop does check against the end of the
> pidmap array properly (it's only the actual pointer arithmetic overflow
> case we need to worry about, and going one bit beyond isn't going to
> overflow).
>
> [ Use PID_MAX_LIMIT rather than pid_max as per Eric Biederman ]
>
> Reported-by: Tavis Ormandy<taviso at cmpxchg8b.com>
> Analyzed-by: Robert Święcki<robert at swiecki.net>
> Cc: Eric W. Biederman<ebiederm at xmission.com>
> Cc: Pavel Emelyanov<xemul at openvz.org>
> Signed-off-by: Linus Torvalds<torvalds at linux-foundation.org>
> (backported from commit c78193e9c7bcbf25b8237ad0dec82f805c4ea69b upstream)
> Signed-off-by: Herton Ronaldo Krzesinski<herton.krzesinski at canonical.com>
> ---
>   .../openvz/patchset/0001-2.6.24-ovz002.patch       |    2 +-
>   kernel/pid.c                                       |    5 ++++-
>   2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/debian/binary-custom.d/openvz/patchset/0001-2.6.24-ovz002.patch b/debian/binary-custom.d/openvz/patchset/0001-2.6.24-ovz002.patch
> index 729b278..6a8a613 100644
> --- a/debian/binary-custom.d/openvz/patchset/0001-2.6.24-ovz002.patch
> +++ b/debian/binary-custom.d/openvz/patchset/0001-2.6.24-ovz002.patch
> @@ -62556,7 +62556,7 @@ Index: kernel/kernel/pid.c
>   +	return pid;
>   +}
>   +
> - static int next_pidmap(struct pid_namespace *pid_ns, int last)
> + static int next_pidmap(struct pid_namespace *pid_ns, unsigned int last)
>    {
>    	int offset;
>   @@ -198,6 +231,7 @@
> diff --git a/kernel/pid.c b/kernel/pid.c
> index f815455..29f0ac0 100644
> --- a/kernel/pid.c
> +++ b/kernel/pid.c
> @@ -181,11 +181,14 @@ static int alloc_pidmap(struct pid_namespace *pid_ns)
>   	return -1;
>   }
>
> -static int next_pidmap(struct pid_namespace *pid_ns, int last)
> +static int next_pidmap(struct pid_namespace *pid_ns, unsigned int last)
>   {
>   	int offset;
>   	struct pidmap *map, *end;
>
> +	if (last>= PID_MAX_LIMIT)
> +		return -1;
> +
>   	offset = (last + 1)&  BITS_PER_PAGE_MASK;
>   	map =&pid_ns->pidmap[(last + 1)/BITS_PER_PAGE];
>   	end =&pid_ns->pidmap[PIDMAP_ENTRIES];

Acked-by: Brad Figg <brad.figg at canonical.com>

-- 
Brad Figg brad.figg at canonical.com http://www.canonical.com