Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751659AbdFEIjO (ORCPT ); Mon, 5 Jun 2017 04:39:14 -0400 Received: from m12-16.163.com ([220.181.12.16]:34223 "EHLO m12-16.163.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751506AbdFEIjM (ORCPT ); Mon, 5 Jun 2017 04:39:12 -0400 Message-ID: <5935190F.7090708@163.com> Date: Mon, 05 Jun 2017 16:40:47 +0800 From: Jia-Ju Bai User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:11.0) Gecko/20120410 Thunderbird/11.0.1 MIME-Version: 1.0 To: Moni Shoua CC: yuval.shaia@oracle.com, Sean Hefty , Doug Ledford , Hal Rosenstock , Leon Romanovsky , linux-rdma , Linux Kernel Mailinglist Subject: Re: [PATCH V3] rxe: Fix a sleep-in-atomic bug in post_one_send References: <1496648342-906-1-git-send-email-baijiaju1990@163.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-CM-TRANSID: EMCowABXXzN5GDVZJqVAIA--.5537S2 X-Coremail-Antispam: 1Uf129KBjvdXoWruFW8KFWxZrW5WF4fXF4UCFg_yoWfZrgEyr 1DtFWvkFy3XFnrK3ZI9395WFW5KFn3Jr1kJa4YvrW3AFn5Xwn7Xa10ywn8W3y8Crs7GFs0 kr1vqF92qw1SvjkaLaAFLSUrUUUUUb8apTn2vfkv8UJUUUU8Yxn0WfASr-VFAUDa7-sFnT 9fnUUvcSsGvfC2KfnxnUUI43ZEXa7IUeStC7UUUUU== X-Originating-IP: [166.111.70.19] X-CM-SenderInfo: xedlyx5dmximizq6il2tof0z/1tbiYxvtelaDtNrVUgAAsT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1145 Lines: 23 On 06/05/2017 04:30 PM, Moni Shoua wrote: >> - if (qp->is_user&& copy_from_user(p, (__user void *) >> - (uintptr_t)sge->addr, sge->length)) >> + spin_unlock_irqrestore(&qp->sq.sq_lock, *flags); >> + err = copy_from_user(p, (__user void *) >> + (uintptr_t)sge->addr, sge->length); >> + spin_lock_irqsave(&qp->sq.sq_lock, *flags); >> + if (qp->is_user&& err) >> return -EFAULT; > qp-_is_user is always false in this function (flow starts from > rxe_post_send_kernel) so this line is a dead code > In fact, this patch seems to add a serious bug when it uses > copy_from_user() from a non user pointer. > Do you agree? I agree. So, it is fine to me to remove this line, as you said in the former email: > Second, I think that there is no flow that leads to this function > when qp->is user is true so maybe the correct action is to remove this > line completely > if (qp->is_user&& copy_from_user(p, (__user void *)