MIME-Version: 1.0
In-Reply-To: <20160423000530.38b4f1c9@tukaani.org>
References: <1461185746-8017-5-git-send-email-keescook@chromium.org>
	<tip-bf0118dbba9542ceb5d33d4a86830a6c88b0bbf6@git.kernel.org>
	<20160423000530.38b4f1c9@tukaani.org>
Date: Fri, 22 Apr 2016 15:01:46 -0700
Message-ID: <CAGXu5jKG4OszKkprY2fNvxZBHXxFaP8Kqr_7j389ho6mkkyyWA@mail.gmail.com>
Subject: Re: [tip:x86/boot] x86/boot: Make memcpy() handle overlaps
From: Kees Cook <keescook@chromium.org>
To: Lasse Collin <lasse.collin@tukaani.org>
Cc: Andrey Ryabinin <aryabinin@virtuozzo.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Denys Vlasenko <dvlasenk@redhat.com>,
        LKML <linux-kernel@vger.kernel.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        "H. Peter Anvin" <hpa@zytor.com>, Andy Lutomirski <luto@kernel.org>,
        Yinghai Lu <yinghai@kernel.org>, Josh Poimboeuf <jpoimboe@redhat.com>,
        Borislav Petkov <bp@suse.de>, Dmitry Vyukov <dvyukov@google.com>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Ingo Molnar <mingo@kernel.org>, Baoquan He <bhe@redhat.com>,
        Thomas Gleixner <tglx@linutronix.de>, Borislav Petkov <bp@alien8.de>,
        Brian Gerst <brgerst@gmail.com>, Andy Lutomirski <luto@amacapital.net>,
        "H.J. Lu" <hjl.tools@gmail.com>
Content-Type: text/plain; charset=UTF-8
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1239
Lines: 34

On Fri, Apr 22, 2016 at 2:05 PM, Lasse Collin <lasse.collin@tukaani.org> wrote:
> On 2016-04-22 tip-bot for Kees Cook wrote:
>> x86/boot: Make memcpy() handle overlaps
>>
>> Two uses of memcpy() (screen scrolling and ELF parsing) were handling
>> overlapping memory areas. While there were no explicitly noticed bugs
>> here (yet), it is best to fix this so that the copying will always be
>> safe.
>>
>> Instead of making a new memmove() function that might collide with
>> other memmove() definitions in the decompressors, this just makes the
>> compressed boot code's copy of memcpy() overlap-safe.
>
> So far lib/decompress_unxz.c is the only decompressor that needs
> memmove(). There the local definition is inside #ifndef to make it easy
> to omit it and to use another memmove() implementation. It's enough to
> do this:
>
>     #define memmove memmove
>
> To me it sounds less confusing if a function that works on overlapping
> buffers is named memmove() instead of memcpy(). In those places where
> buffers can overlap one would then use memmove() so that it's clear to
> the reader that overlapping is possible.

Okay, I'll refactor this and double-check the xz case.

Thanks!

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security