From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id AF124C433F5 for ; Thu, 31 Mar 2022 18:04:05 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233365AbiCaSFv (ORCPT ); Thu, 31 Mar 2022 14:05:51 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36784 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232596AbiCaSFu (ORCPT ); Thu, 31 Mar 2022 14:05:50 -0400 Received: from mail-pf1-x431.google.com (mail-pf1-x431.google.com [IPv6:2607:f8b0:4864:20::431]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A3C7843391 for ; Thu, 31 Mar 2022 11:04:01 -0700 (PDT) Received: by mail-pf1-x431.google.com with SMTP id b15so314996pfm.5 for ; Thu, 31 Mar 2022 11:04:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=message-id:date:mime-version:user-agent:content-language:to:cc :references:from:subject:in-reply-to:content-transfer-encoding; bh=YsjSmrG2rvYs50asz/taC5lCuB0Ss1bBZU3bReqw5is=; b=PouxUIYkBVuDtFYYhzJKLOVGEietOc0cr9qJm2EvDslOhzSw9jON2ICmYF6t9N5/6d UeUa90pA/MozGMKak7kSYV0mXp43DAEGF2q7tXAYxPBwHknQABhfGpVQXvnSm8QSTt/n Eok9vzYTaPRKddP8OxP4oaKDkzdCIH7LL/RmyFAUWgF7jsxDViqNjjb7WbU/DZKVxRtG fN7/0u+gUYr79gPq5Rro0aSzY01+eXTZQ0BBw8iACDlZqN9rtqzBBl4iJe/qiOdvTHpe ywKFz8znw4luKvpXbOv6m6ae8EobDnAtXSAX89rWDWgXE8ZobRk7ujXauAwBCNcXwkU9 yg7g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent :content-language:to:cc:references:from:subject:in-reply-to :content-transfer-encoding; bh=YsjSmrG2rvYs50asz/taC5lCuB0Ss1bBZU3bReqw5is=; b=gwIquj0QP0WLK2Uju5L2Tw6ssrIhcw3pEFC/Emip/c/XVlI20MDkJJWWWReOXN+S2p MAEvDTnpYcp/NcIHWITnH6A6CrEqolVV4llrTIOV7/dlGkeZRj5JuqSDES7kJx9rz9xj juRWMfYmxxhW3FWKV6r01rGF+gE7iPLeJC2f1fryZphUq0FQuLt1Cw7FDAszuWvsjk9F wLDdLzWCo3xB0ZppccYEcTIt8QCf4MWu93vq6/BREqglh3ghcQ31zO/0WrCdKAX+q0xd 3PCkfNdFY8zTM8HcO+/RHexVdkSUKmj//vomqa2tOxsbKnTpWLIT981khHoPzUmRttKV hk1A== X-Gm-Message-State: AOAM530RPiXNu4W69aobYky7x4ezezpmJa3iM+Cn0HWtxSBPBN7pOj6y b7IgeapmDvX5R64IcpLyDDRM/A== X-Google-Smtp-Source: ABdhPJzs/UIaReCHNW6gfF+eZ6FRhildRbC1WSzb8rJwa2DI3Yesk+oxp4qzOcQkzybMD/Ht6oeMLA== X-Received: by 2002:a65:6d15:0:b0:382:4e6d:dd0d with SMTP id bf21-20020a656d15000000b003824e6ddd0dmr11580133pgb.333.1648749841056; Thu, 31 Mar 2022 11:04:01 -0700 (PDT) Received: from [192.168.254.17] ([50.39.160.154]) by smtp.gmail.com with ESMTPSA id oc10-20020a17090b1c0a00b001c7510ed0c8sm11299522pjb.49.2022.03.31.11.03.59 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 31 Mar 2022 11:04:00 -0700 (PDT) Message-ID: <39b76458-17e6-4e04-15d8-1445d2067d0c@linaro.org> Date: Thu, 31 Mar 2022 11:03:59 -0700 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.7.0 Content-Language: en-US To: Theodore Ts'o Cc: Andreas Dilger , Ritesh Harjani , linux-ext4@vger.kernel.org, stable@vger.kernel.org, linux-kernel@vger.kernel.org, syzbot+7a806094edd5d07ba029@syzkaller.appspotmail.com References: <20220315215439.269122-1-tadeusz.struk@linaro.org> From: Tadeusz Struk Subject: Re: [PATCH v2] ext4: check if offset+length is valid in fallocate In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-ext4@vger.kernel.org On 3/31/22 07:43, Theodore Ts'o wrote: > On Tue, Mar 15, 2022 at 02:54:39PM -0700, Tadeusz Struk wrote: >> @@ -3967,6 +3968,16 @@ int ext4_punch_hole(struct inode *inode, loff_t offset, loff_t length) >> offset; >> } >> >> + /* >> + * For punch hole the length + offset needs to be at least within >> + * one block before last >> + */ >> + max_length = sbi->s_bitmap_maxbytes - inode->i_sb->s_blocksize; >> + if (offset + length >= max_length) { >> + ret = -ENOSPC; >> + goto out_mutex; >> + } > > I wonder if we would be better off just simply capping length to > max_length? If length is set to some large value, such as LONG_MAX, > it's pretty clear what the intention should be, which is to simply do > the equivalent of truncating the file at offset. Perhaps we should > just do that? Don't think that would be the correct behavior. ftrucnate (or truncate) modify the file size, but fallocate with FALLOC_FL_PUNCH_HOLE should not. man 2 fallocate says: "... The FALLOC_FL_PUNCH_HOLE flag must be ORed with FALLOC_FL_KEEP_SIZE in mode; in other words, even when punching off the end of the file, the file size (as reported by stat(2)) does not change. " that is enforced by vfs: https://elixir.bootlin.com/linux/v5.17.1/source/fs/open.c#L245 > > That being said, we should be consistent with what other file systems > do when they are asked to punch a hole starting at offset and > extending out to LONG_MAX. For all the supported file systems, apart from ext4, only btrfs, gfs2, and xfs support fallocate and FALLOC_FL_PUNCH_HOLE mode. Looking at what they do is they round the length of the space to be freed i.e. offset + length to valid value and then perform the operation using the valid values. https://elixir.bootlin.com/linux/v5.17.1/source/fs/gfs2/bmap.c#L2424 https://elixir.bootlin.com/linux/v5.17.1/source/fs/btrfs/file.c#L2506 For ext4 this would mean that one could only deallocate space up to the one before last block. I will change this to do the same in the next version if that's ok with you. > > Also, if we are going to return an error, I don't think ENOSPC is the > correct error to be returning. I took it from man 2 fallocate, my first suspicion was that it crashed because the disk on my VM wasn't big enough. It was a bad choice. -- Thanks, Tadeusz