From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f43.google.com (mail-wm1-f43.google.com [209.85.128.43]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 5CA30403E8E for ; Tue, 19 May 2026 15:15:47 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.43 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779203749; cv=none; b=MmHiVuYqWFSvhvHn5J7cDrajCH0eoC31GkFyRYepuVd5eBuGUUfzSfOoIqOaBDY8MLllV6gmMvQQqCsbzz8jMlYSH/N2aAmsiGwOwtUSlnrV9mhtGf1TBAbveEwNQxidQyJszKclTyZ9flYlk6tjX0sv8htryLadoWUJA3jifLg= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779203749; c=relaxed/simple; bh=SNNvpQQuPVQMvp44GkMMtPNiqpHye4xFnZSe12UszYw=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=TNBYuHJsVBOkPivR5eUMABas834/MNnw+1gnQtyykoJ97nxEMbKNTfqDoqWQm7B6AVxo2c0IeuJRuuXvqDZrc32FSqkqWB9rEYPbPcz8PeLsv+N72+T7d0TFXJG93j7y/6ff1mYgwh87aqKJhDBd7dPM2XCYTTjd4ytZ8HFsr3Y= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=szuAxYan; arc=none smtp.client-ip=209.85.128.43 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="szuAxYan" Received: by mail-wm1-f43.google.com with SMTP id 5b1f17b1804b1-48d146705b4so40486565e9.3 for ; Tue, 19 May 2026 08:15:47 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1779203746; x=1779808546; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=BdiflLEoOIW6130918XzLUR22FM/UxzpgmqRMxrhy/I=; b=szuAxYanJnRbJP7a+0A3VbyKLZhktq0QsWpb/pPSV/mefNbBU5UXjjjKABMfRdeMPY CirvF4RxlYYYh7AWiWspkr1ZCWc49zfqmTsxg5FHwkvxRgKVapFXlNKrF7eayWH0LKEy EqbhjC1e0F28K1xWyRoyPg0fqtF11iLN+g1P8TvX7QMpFIohkQAsoJuea4cFvOA2THFo IUYrk/eZo7nybZCl1wPCW8Xf90J4SZd7MiasJKGZvH9vO0mc1Li5TZyhFboOmCJatUWS 3urWG8r1baH8ToqNVgliwBmhE7ipiG7SNSO4W5CU9EJKzeV/J2yvZ/fgNEeLQjRHDVmI H6kg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1779203746; x=1779808546; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=BdiflLEoOIW6130918XzLUR22FM/UxzpgmqRMxrhy/I=; b=l9k34aOmQYsZiAxBo5w2HVEpcexxrZtC1BRwlsrY0VtKZCGne7RK6KvayGqOK345q9 MWit52NL9aifpoA/R4I/Z3L2BvsaNZBNxGeAaQgnIv4kOdjeWkB1q2QN5Uevnyygygee 8iv0SFdB/cd1y8nGXq8DqKjSmchrpAzXULYYStQi8yk2bfI1VrznqlGxifY2UvA9UygY 5g4meQztzOOpxQM+rhfPbBBCczUEw9Ld30WmV9gaKm/Bqcnzl5P0B0DYPqIJNsQUZbLl HXGgksaVvDJTEQuvcsyO+iyInXvjLsGXAO2TFh7OO/XeOXkiSVjV2+0yZmhLGDmVMig8 zFSA== X-Forwarded-Encrypted: i=1; AFNElJ+NrAuiHmR4kIf1ud/WEAWSXFxv+ZKglaT1mSxPdi4Yd3YmkRJPfaYYur0GiwUqss6nJoxjZVUO3DKHld8=@vger.kernel.org X-Gm-Message-State: AOJu0YyVP60zdMTUxmYK5ynv6xTY6RVINMttnkS99iNZ1gF4ATRpt7um ogYWkxfe4tvT4gJt7pVG2qM7fvOXreUUMUffEFfScCJqlZcikWHTkEKM X-Gm-Gg: Acq92OEhqM6wjHvJsIJXj4xT5UD5ykBUn9OEZhdRLFisrnyQn4LEXHkn/+bJ/w8O0hw A+H17EmYoHHWzQOauBkUE9hWDP6lOMouUCuSUjKHNhQUvHBtLx0Y6D8B8NWFSBvgX3hgcFgF+YQ 3FQFa12JlqHT6llQrYE7tookDc9IGKz5rV2wkY0gBhdQHgHPIJDfWYZcDJ5ouXgcb8dfqdpJ9Fj 5ck7T6b838609E3oneJUrsz/RTRpYmPC0TDeIohhtj9WxIgutJaOzzIkYB2g+J4CpM/+y4w0llx OB0tNF+kVL++TcBKltIlLBalmnGAWDZeNJs734KMe5cqRrqNjoTjOEHSqIa5VBNKXtnv3lv05Nu tgLi1hM38U3GwQBmeecwSAYkZLEgxbKLMieSTwEIsvjHn2bthJHtd6PrsMsy5mf4mb7eDP9AvIU Aq+ZstSUDEqWL3wdYTcL2bImwIs68Wuc9FN1SPbySPM870M1IDUskFnh3WiKGGI3DgzRN6niJFX isRggsH60c= X-Received: by 2002:a05:600c:6098:b0:48f:d1b8:9ab1 with SMTP id 5b1f17b1804b1-48fe60ecc51mr346949665e9.9.1779203746282; Tue, 19 May 2026 08:15:46 -0700 (PDT) Received: from ?IPV6:2a01:4b00:bd1f:f500:f867:fc8a:5174:5755? ([2a01:4b00:bd1f:f500:f867:fc8a:5174:5755]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-48fe5ab3977sm363119485e9.9.2026.05.19.08.15.44 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 19 May 2026 08:15:45 -0700 (PDT) Message-ID: Date: Tue, 19 May 2026 16:15:44 +0100 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH bpf-next 2/5] bpf: Fix concurrent regression in map_create() To: Leon Hwang , bpf@vger.kernel.org Cc: Alexei Starovoitov , Daniel Borkmann , Andrii Nakryiko , Martin KaFai Lau , Eduard Zingerman , Kumar Kartikeya Dwivedi , Shuah Khan , linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org, kernel-patches-bot@fb.com References: <20260518145446.6794-1-leon.hwang@linux.dev> <20260518145446.6794-3-leon.hwang@linux.dev> <261a47f4-a92b-4a61-86ca-c1de362be105@gmail.com> <5660e657-f2a5-4866-9881-c705bff0c970@linux.dev> Content-Language: en-US From: Mykyta Yatsenko In-Reply-To: <5660e657-f2a5-4866-9881-c705bff0c970@linux.dev> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit On 5/19/26 3:47 AM, Leon Hwang wrote: > On 19/5/26 00:43, Mykyta Yatsenko wrote: >> >> >> On 5/18/26 3:54 PM, Leon Hwang wrote: > [...] >>> /* preserve original error even if log finalization is successful */ >>> ret = bpf_log_attr_finalize(&attr_log, log); >>> - if (ret) { >>> - if (err >= 0) >>> - close_fd(err); >>> + if (ret && err < 0) >>> + /* >>> + * Failed to finalize the log. >>> + * Should not close_fd(err) here. Since the bpf_map_new_fd() >> >> nit: you can't close_fd(err) here, because err < 0? The comment overall appears to >> explain why we are making this change right now, rather than why this works this way. >> > > Will update the comment to > > /* > * Failed to finalize the log. > * > * Should not close_fd(err) here by > * > * if (ret) { > * if (err >= 0) > * close_fd(err); > * err = ret; > * } > * > * Since the bpf_map_new_fd() has published the map fd, > * if a concurrent thread closes the fd, then opens new, > * unrelated file that receives the exact same fd > * number, close_fd(err) might inadvertently close the > * unrelated file. > * > * As a trade-off, override the err only when failed to > * finalize the log and failed to create map. > */ > > The comment is to explain the reason for overriding err. > >> If map_crate() failed with error code and then log finalization failed, do we really >> want to override error code? It sounds like map_create error is more important. >> > > In that case, there are two error codes that should be returned to user > space. How to achieve it? > > Since we should not ignore any error code of them, can we report the > error code of __map_create() failure by adding an error attr to struct > bpf_common_attr? Hmm, seems not a good idea. > > Any idea? I think we should do what Alexei suggests. For example: we return ENOSPC when log is too short, regardless of the status of the operation for BPF_PROG_LOAD (see bpf_object_load_prog() in libbpf.c), it makes sense to do it consistently for map_create as well. log_finalize() error always overrides map_create() error. A precedent: We are safe from this race in bpf_prog_load(): bpf_prog_alloc_id() makes prog exposed, only after bpf_check() succeeds, which finalizes log. We should do similar for map_create(). > > Thanks, > Leon > >>> + * has published the map fd, if a concurrent thread closes the >>> + * fd, then opens new, unrelated file that receives the exact >>> + * same fd number, close_fd(err) might inadvertently close the >>> + * unrelated file. >>> + * As a trade-off, override the err only when failed to finalize >>> + * the log and failed to create map. >>> + */ >>> err = ret; >>> - } >>> >>> kfree(log); >>> return err; >> >