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 70F38C433EF for ; Sat, 26 Mar 2022 22:15:45 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229741AbiCZWRT (ORCPT ); Sat, 26 Mar 2022 18:17:19 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40464 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229539AbiCZWRQ (ORCPT ); Sat, 26 Mar 2022 18:17:16 -0400 Received: from mail-lf1-x12f.google.com (mail-lf1-x12f.google.com [IPv6:2a00:1450:4864:20::12f]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 3487CE20 for ; Sat, 26 Mar 2022 15:15:39 -0700 (PDT) Received: by mail-lf1-x12f.google.com with SMTP id t25so18934357lfg.7 for ; Sat, 26 Mar 2022 15:15:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux-foundation.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=646eEJy3R0M/KTVuEwo8ILglC9PMb/NEGSld3GpAxEM=; b=VmaZt/FdSU9IxWnJT+n7WMNSAm9QhaWnXKkwGBa+wTkkHVbNrq8oqd2DQI4iCl0wG/ iy7E3AujkyX5NaVU4BcnOT1fn+xKdpfC9DwBzr+NshauWtc6lfNeDSfelnnG9h7FbBG8 I7KA+6jENZZ4Gnz9h8WRtq8IQ0/ZxgCw4o6Ug= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=646eEJy3R0M/KTVuEwo8ILglC9PMb/NEGSld3GpAxEM=; b=3EQJGZ94lK/DxUPJlk9PR8rrwRbFzCsp+LONeYvHfBsC+irTLxMkeM6YY1/05xlTkM p+xUI1g7tThctWHbB6KyyJ7FsyFo2FKijM1QiYbVwxvFSwM39M+pS/rC2tU8a/IIeRmk /KSDHjLTcwublUGlbiI1bPLicga4WEhKINZrK7zoPL8WTfYrKsiFJNzfFmfDH1PTMkr3 87ZsCPZP+zdi3oRj5yPIEcjm4HN5DfSBTiVRmlqEp1gQBaJCsbqwVZPZGCF51RtDkz6j K4tHTLrBX68D9grjKORdmS++zklZaLa+1WBPIjDdHm8XwqG7cXeejxh+HO/BvnOy9xqs xvWw== X-Gm-Message-State: AOAM530TISop6v6Djcwj1eGx1ICTqcEc1u83tY/4QpIFNSq0Dcvw5TWu Qu10g28Jo1SJOI8JNtxQcj3RocYlbuWkgS1OU1Q= X-Google-Smtp-Source: ABdhPJxFyIWjeQP6YO+0CSsTBV3yu3Y+jSiLStvbxgs4nxWgwskvrsG3b/6QYAM6WJCt0fYwzI1YVg== X-Received: by 2002:a05:6512:6c1:b0:448:6291:f135 with SMTP id u1-20020a05651206c100b004486291f135mr13437489lff.451.1648332937042; Sat, 26 Mar 2022 15:15:37 -0700 (PDT) Received: from mail-lj1-f177.google.com (mail-lj1-f177.google.com. [209.85.208.177]) by smtp.gmail.com with ESMTPSA id o7-20020ac24c47000000b0044a15c4e0aesm1182821lfk.272.2022.03.26.15.15.35 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sat, 26 Mar 2022 15:15:36 -0700 (PDT) Received: by mail-lj1-f177.google.com with SMTP id v12so1836831ljd.3 for ; Sat, 26 Mar 2022 15:15:35 -0700 (PDT) X-Received: by 2002:a2e:9b10:0:b0:247:f28c:ffd3 with SMTP id u16-20020a2e9b10000000b00247f28cffd3mr13362606lji.152.1648332935415; Sat, 26 Mar 2022 15:15:35 -0700 (PDT) MIME-Version: 1.0 References: <20220326114009.1690-1-aissur0002@gmail.com> In-Reply-To: From: Linus Torvalds Date: Sat, 26 Mar 2022 15:15:19 -0700 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH 4/4] file: Fix file descriptor leak in copy_fd_bitmaps() To: Alexey Khoroshilov , Eric Biggers , Christian Brauner Cc: Fedor Pchelkin , Alexander Viro , linux-fsdevel , Linux Kernel Mailing List Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Sorry, quoting everything below to bring in Eric Biggers because he touched that particular code last. And Christian Brauner, because he worked on all teh bitmap code with the whole close_range thing. I think this is all ok because the number of files aren't just byte-aligned, they are long-aligned: * We make sure that nr remains a multiple of BITS_PER_LONG - otherwise * bitmaps handling below becomes unpleasant, to put it mildly... but maybe I'm missing something. The fact that there's a Found by Syzkaller (https://github.com/google/syzkaller). thing in that suggested commit message makes me think there _is_ something I'm missing. Certainly NR_OPEN_DEFAULT, sane_fdtable_size() and max_fds should always be a multiple of BITS_PER_LONG. So I don't _think_ there is any bug here, although it might be good to (a) document that "we explicitly do things in BITS_PER_LONG chunks" even more in places (b) have people double-check my thinking because clearly that syzcaller thing implies I'm full of crap Eric, Christian? Can somebody point to the actual syzkaller report? Linus On Sat, Mar 26, 2022 at 7:17 AM Alexey Khoroshilov wrote: > > Looks like bfp has a set of macro suitable for such cases: > > #define BITS_PER_BYTE_MASKED(bits) ((bits) & BITS_PER_BYTE_MASK) > #define BITS_ROUNDDOWN_BYTES(bits) ((bits) >> 3) > #define BITS_ROUNDUP_BYTES(bits) \ > (BITS_ROUNDDOWN_BYTES(bits) + !!BITS_PER_BYTE_MASKED(bits)) > > May be it makes sense to move them to a generic header and to use here? > > -- > Alexey Khoroshilov > > > On 26.03.2022 14:40, Fedor Pchelkin wrote: > > If count argument in copy_fd_bitmaps() is not a multiple of > > BITS_PER_BYTE, then one byte is lost and is not used in further > > manipulations with cpy value in memcpy() and memset() > > causing a leak. The leak was introduced with close_range() call > > using CLOSE_RANGE_UNSHARE flag. > > > > The patch suggests implementing an indicator (named add_byte) > > of count being multiple of BITS_PER_BYTE and adding it to the > > cpy value. > > > > Found by Syzkaller (https://github.com/google/syzkaller). > > > > Signed-off-by: Fedor Pchelkin > > Signed-off-by: Alexey Khoroshilov > > --- > > fs/file.c | 2 -- > > 1 file changed, 2 deletions(-) > > > > diff --git a/fs/file.c b/fs/file.c > > index 3ef1479df203..3c64a6423604 100644 > > --- a/fs/file.c > > +++ b/fs/file.c > > @@ -56,10 +56,8 @@ static void copy_fd_bitmaps(struct fdtable *nfdt, struct fdtable *ofdt, > > { > > unsigned int cpy, set; > > unsigned int add_byte = 0; > > - > > if (count % BITS_PER_BYTE != 0) > > add_byte = 1; > > - > > cpy = count / BITS_PER_BYTE + add_byte; > > set = (nfdt->max_fds - count) / BITS_PER_BYTE; > > memcpy(nfdt->open_fds, ofdt->open_fds, cpy); > > >