Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752927AbeABOwQ (ORCPT + 1 other); Tue, 2 Jan 2018 09:52:16 -0500 Received: from bombadil.infradead.org ([65.50.211.133]:59038 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751392AbeABOwO (ORCPT ); Tue, 2 Jan 2018 09:52:14 -0500 Date: Tue, 2 Jan 2018 06:51:55 -0800 From: Matthew Wilcox To: "Leizhen (ThunderTown)" Cc: Jeff Moyer , Alexander Viro , Benjamin LaHaise , linux-fsdevel , linux-aio , linux-kernel , Tianhong Ding , Hanjun Guo , Libin , Kefeng Wang , Deepa Dinamani Subject: Re: [PATCH 1/1] aio: make sure the input "timeout" value is valid Message-ID: <20180102145155.GD8222@bombadil.infradead.org> References: <1513172572-16724-1-git-send-email-thunder.leizhen@huawei.com> <20171213141112.GA11217@bombadil.infradead.org> <20171213193100.GA19700@bombadil.infradead.org> <5A31ED86.5000800@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <5A31ED86.5000800@huawei.com> User-Agent: Mutt/1.9.1 (2017-09-22) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Return-Path: On Thu, Dec 14, 2017 at 11:18:30AM +0800, Leizhen (ThunderTown) wrote: > On 2017/12/14 3:31, Matthew Wilcox wrote: > > On Wed, Dec 13, 2017 at 11:27:00AM -0500, Jeff Moyer wrote: > >> Matthew Wilcox writes: > >> > >>> On Wed, Dec 13, 2017 at 09:42:52PM +0800, Zhen Lei wrote: > >>>> Below information is reported by a lower kernel version, and I saw the > >>>> problem still exist in current version. > >>> > >>> I think you're right, but what an awful interface we have here! > >>> The user must not only fetch it, they must validate it separately? > >>> And if they forget, then userspace is provoking undefined behaviour? Ugh. > >>> Why not this: > >> > >> Why not go a step further and have get_timespec64 check for validity? > >> I wonder what caller doesn't want that to happen... > I tried this before. But I found some places call get_timespec64 in the following function. > If we do the check in get_timespec64, the check will be duplicated. > > For example: > static long do_pselect(int n, fd_set __user *inp, fd_set __user *outp, > .... > if (get_timespec64(&ts, tsp)) > return -EFAULT; > > to = &end_time; > if (poll_select_set_timeout(to, ts.tv_sec, ts.tv_nsec)) > > int poll_select_set_timeout(struct timespec64 *to, time64_t sec, long nsec) > { > struct timespec64 ts = {.tv_sec = sec, .tv_nsec = nsec}; > > if (!timespec64_valid(&ts)) > return -EINVAL; The check is only two comparisons! Why do we have an interface that can cause bugs for the sake of saving *two comparisons*?! Can we talk about the cost of a cache miss versus the cost of executing these comparisons?