From mboxrd@z Thu Jan 1 00:00:00 1970 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965229AbeALTth (ORCPT + 1 other); Fri, 12 Jan 2018 14:49:37 -0500 Received: from mx1.redhat.com ([209.132.183.28]:45586 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753875AbeALTtg (ORCPT ); Fri, 12 Jan 2018 14:49:36 -0500 From: Jeff Moyer To: Matthew Wilcox Cc: "Leizhen \(ThunderTown\)" , 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 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> <20180102145155.GD8222@bombadil.infradead.org> X-PGP-KeyID: 1F78E1B4 X-PGP-CertKey: F6FE 280D 8293 F72C 65FD 5A58 1FF8 A7CA 1F78 E1B4 X-PCLoadLetter: What the f**k does that mean? Date: Fri, 12 Jan 2018 14:49:33 -0500 In-Reply-To: <20180102145155.GD8222@bombadil.infradead.org> (Matthew Wilcox's message of "Tue, 2 Jan 2018 06:51:55 -0800") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Fri, 12 Jan 2018 19:49:36 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Return-Path: Matthew Wilcox writes: > 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? Any update on this? Willy, I'd be okay with your get_valid_timespec64 patch if you wanted to formally submit that. -Jeff