Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753024AbbHUAFt (ORCPT ); Thu, 20 Aug 2015 20:05:49 -0400 Received: from mail-io0-f179.google.com ([209.85.223.179]:35671 "EHLO mail-io0-f179.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751700AbbHUAFs (ORCPT ); Thu, 20 Aug 2015 20:05:48 -0400 MIME-Version: 1.0 In-Reply-To: <1440061141-9778-1-git-send-email-yalin.wang2010@gmail.com> References: <1440061141-9778-1-git-send-email-yalin.wang2010@gmail.com> Date: Thu, 20 Aug 2015 17:05:47 -0700 X-Google-Sender-Auth: CSeuNDQJowvqrKHVOxMaTMXuyX0 Message-ID: Subject: Re: [RFC] fs/kcore: change copy_to_user to copy_in_user From: Linus Torvalds To: yalin wang Cc: Borislav Petkov , Ingo Molnar , Dave Hansen , bhe@redhat.com, Andrew Morton , Linux Kernel Mailing List Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1875 Lines: 42 On Thu, Aug 20, 2015 at 1:59 AM, yalin wang wrote: > - > - n = copy_to_user(buffer, (char *)start, tsz); > + if ((start + tsz < tsz) || > + (start + tsz) > TASK_SIZE) > + return -EFAULT; This is wrong. You apparently want to have if (!access_ok(start, tsz)) return -EFAULT; > + set_fs(KERNEL_DS); > + n = copy_in_user(buffer, (char *)start, tsz); > + set_fs(USER_DS); .. and this is actually worse and even less portable than what we have now, in that it's actively wrong on platforms that may have a user address and a kernel address with the same value (ie they have explicitly separate kernel/user address spaces). Now, that's admittedly unusual, but I think sparc32 actually can do that. Anyway, I absolutely detest this patch. It replaces one piece of code that admittedly doesn't work on all architectures because kernel memory is accessed without testing, with another hack that happens to work on other architectures and is fragile and prone to be a security issue. In other words, I think the end result is _worse_ than the current situation. You probably want to use "probe_kernel_read()" and do it into a temporary buffer, and then just do the copy_to_user() from the temporary buffer. Sure, it's less efficient, but at least it's not actively wrong and a possible security problem in the long run. Linus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/