[next] media: venus: hfi_cmds: Replace one-element array with flex-array member and use __counted_by
Message ID | ZSRJfRdUXQOzagKr@work |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:a888:0:b0:403:3b70:6f57 with SMTP id x8csp2057154vqo; Mon, 9 Oct 2023 11:42:50 -0700 (PDT) X-Google-Smtp-Source: AGHT+IFhUXVP0QeGK9T4hqfau5Qx8U/Q7Et3en0rjrF/fd/QTH/bh2RpQ+6H8rYECTMkJfqDr/Ns X-Received: by 2002:a17:903:11c8:b0:1c7:2697:ec10 with SMTP id q8-20020a17090311c800b001c72697ec10mr18902326plh.56.1696876970368; Mon, 09 Oct 2023 11:42:50 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1696876970; cv=none; d=google.com; s=arc-20160816; b=ATK7e0WKqDEDzDOyMYGvfCLsjqUKwluMGYtiDfIRAjeVsfuBvsd7VRCQcuZ2KNY2GR hXHhpinv9TYf0cmek3kVKJJWKRLWfrv9amldZiYoyj4GgN6ADCbwd/iaRpDIyWSsuf4t Hbg5TI1le/pIwg7eNl/gsX6z/Gl4LPCjKe/6oCP8JjjrVmsq387kRqlbgf5v61MKmxxz YHErPyzj77McP3PGCpsaTdyV+S0p8rAFbm+0C05xmZygLq4VhaqkzXRKXiJe9snTQHBG m2xsyjpalmWaiomSzrJfuRgm3E3cOC6v8cwfr0ZeBSYYGy5U77Kz3PwtQ5s6OfJ/emmX 7zig== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-disposition:mime-version:message-id :subject:cc:to:from:date:dkim-signature; bh=+IyRiWickrRcdiRvAWGvXsQqczM7mcNHFN+SU13T9J4=; fh=V1zW8Pj/gMgNWT25ZOO4qkTGt2oVWUuqtm1Eggufv8Q=; b=HiotE89I9orwT1lMV1QA4ZVxxgfxuzjLCo7WklV6EvOEuHrgCfimAEA37ebaukxn1J 0zcoF3H6dDbFIxauhlf3+Fd0xb0IZ+E+Q+rqUP8TtMFlPM6oOjiJHf10PtWtmF6tng9P ui8n6azp7PvVuXUPqtjodd3tJOBHpJ+U7oGvbUVaKTBy8QSfEnAesYdO7UdcC9D5z+je xVQztiiaCpxF5IUXL/VWOzD0Rfw23/KBbqPeRgubtzzaWfqPa7I9UoeeUuQq6fw+EVyC dMWmP03ahwk6kgqxKRH8r8ipNkGz6JGIpBa04bsg/rtiJLInHLzcjf+Jvi2a9FyB42pv RJNg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=dUdg+Q1G; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: from fry.vger.email (fry.vger.email. [23.128.96.38]) by mx.google.com with ESMTPS id q1-20020a170902a3c100b0019c354055d0si9509760plb.304.2023.10.09.11.42.49 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 09 Oct 2023 11:42:50 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) client-ip=23.128.96.38; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=dUdg+Q1G; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by fry.vger.email (Postfix) with ESMTP id 17C2181CA3F6; Mon, 9 Oct 2023 11:42:48 -0700 (PDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at fry.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1378056AbjJISmh (ORCPT <rfc822;ezelljr.billy@gmail.com> + 18 others); Mon, 9 Oct 2023 14:42:37 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46830 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1378344AbjJISm2 (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Mon, 9 Oct 2023 14:42:28 -0400 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id EF5631A4; Mon, 9 Oct 2023 11:42:09 -0700 (PDT) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 2863AC433C7; Mon, 9 Oct 2023 18:42:08 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1696876929; bh=pOd28ycSTuXiUWMP0Dwr/hJLjUUz6PJ55hSKuTCzAb4=; h=Date:From:To:Cc:Subject:From; b=dUdg+Q1GNGEu1lG4+5q+dBKPNIXKRXg+XNVDksa9t4B7yiviWQtCOAEYsFdlTq1GV a8c30hvRV+VqasI0wjQscMsnPLVw2PCVmveO1LX2uC+HefZ7rUFM3x4psnjbOJeXsn RVL+eCQwM5t59B+3wZKf8Vk9PcVKwAr1feGMCQ+/X23ehYamX4NMhe4zcxeObIJYuZ m+7y9hN7iXY2bVOnXN+gfipCzwMYDdSZAfUt59Ayq2QGWKEXi0zsKtGEqLqtFZyTtG skqPReNdklQEKY5UJLnCdkpAlsZ1A4FispQQ2TP5Z3w6DYfaRO4BwKYTHH66QaZaTk s7dsg98nFck6Q== Date: Mon, 9 Oct 2023 12:42:05 -0600 From: "Gustavo A. R. Silva" <gustavoars@kernel.org> To: Stanimir Varbanov <stanimir.k.varbanov@gmail.com>, Vikash Garodia <quic_vgarodia@quicinc.com>, Bryan O'Donoghue <bryan.odonoghue@linaro.org>, Andy Gross <agross@kernel.org>, Bjorn Andersson <andersson@kernel.org>, Konrad Dybcio <konrad.dybcio@linaro.org>, Mauro Carvalho Chehab <mchehab@kernel.org> Cc: linux-media@vger.kernel.org, linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org, "Gustavo A. R. Silva" <gustavoars@kernel.org>, linux-hardening@vger.kernel.org Subject: [PATCH][next] media: venus: hfi_cmds: Replace one-element array with flex-array member and use __counted_by Message-ID: <ZSRJfRdUXQOzagKr@work> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Spam-Status: No, score=2.4 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,MAILING_LIST_MULTI, RCVD_IN_SBL_CSS,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on fry.vger.email Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (fry.vger.email [0.0.0.0]); Mon, 09 Oct 2023 11:42:48 -0700 (PDT) X-Spam-Level: ** X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1779304466166552256 X-GMAIL-MSGID: 1779304466166552256 |
Series |
[next] media: venus: hfi_cmds: Replace one-element array with flex-array member and use __counted_by
|
|
Commit Message
Gustavo A. R. Silva
Oct. 9, 2023, 6:42 p.m. UTC
Array `data` in `struct hfi_sfr` is being used as a fake flexible array
at run-time:
drivers/media/platform/qcom/venus/hfi_venus.c:
1033 p = memchr(sfr->data, '\0', sfr->buf_size);
1034 /*
1035 * SFR isn't guaranteed to be NULL terminated since SYS_ERROR indicates
1036 * that Venus is in the process of crashing.
1037 */
1038 if (!p)
1039 sfr->data[sfr->buf_size - 1] = '\0';
1040
1041 dev_err_ratelimited(dev, "SFR message from FW: %s\n", sfr->data);
Fake flexible arrays are deprecated, and should be replaced by
flexible-array members. So, replace one-element array with a
flexible-array member in `struct hfi_sfr`.
While there, also annotate array `data` with __counted_by() to prepare
for the coming implementation by GCC and Clang of the __counted_by
attribute. Flexible array members annotated with __counted_by can have
their accesses bounds-checked at run-time via CONFIG_UBSAN_BOUNDS (for
array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family
functions).
This results in no differences in binary output.
This issue was found with the help of Coccinelle, and audited and fixed
manually.
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
---
drivers/media/platform/qcom/venus/hfi_cmds.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Comments
On Mon, Oct 09, 2023 at 12:42:05PM -0600, Gustavo A. R. Silva wrote: > Array `data` in `struct hfi_sfr` is being used as a fake flexible array > at run-time: > > drivers/media/platform/qcom/venus/hfi_venus.c: > 1033 p = memchr(sfr->data, '\0', sfr->buf_size); > 1034 /* > 1035 * SFR isn't guaranteed to be NULL terminated since SYS_ERROR indicates > 1036 * that Venus is in the process of crashing. > 1037 */ > 1038 if (!p) > 1039 sfr->data[sfr->buf_size - 1] = '\0'; > 1040 > 1041 dev_err_ratelimited(dev, "SFR message from FW: %s\n", sfr->data); > > Fake flexible arrays are deprecated, and should be replaced by > flexible-array members. So, replace one-element array with a > flexible-array member in `struct hfi_sfr`. > > While there, also annotate array `data` with __counted_by() to prepare > for the coming implementation by GCC and Clang of the __counted_by > attribute. Flexible array members annotated with __counted_by can have > their accesses bounds-checked at run-time via CONFIG_UBSAN_BOUNDS (for > array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family > functions). > > This results in no differences in binary output. Thanks for checking! > > This issue was found with the help of Coccinelle, and audited and fixed > manually. > > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> Reviewed-by: Kees Cook <keescook@chromium.org>
On (23/10/09 12:52), Kees Cook wrote: > On Mon, Oct 09, 2023 at 12:42:05PM -0600, Gustavo A. R. Silva wrote: > > Array `data` in `struct hfi_sfr` is being used as a fake flexible array > > at run-time: > > > > drivers/media/platform/qcom/venus/hfi_venus.c: > > 1033 p = memchr(sfr->data, '\0', sfr->buf_size); > > 1034 /* > > 1035 * SFR isn't guaranteed to be NULL terminated since SYS_ERROR indicates > > 1036 * that Venus is in the process of crashing. > > 1037 */ > > 1038 if (!p) > > 1039 sfr->data[sfr->buf_size - 1] = '\0'; > > 1040 > > 1041 dev_err_ratelimited(dev, "SFR message from FW: %s\n", sfr->data); > > > > Fake flexible arrays are deprecated, and should be replaced by > > flexible-array members. So, replace one-element array with a > > flexible-array member in `struct hfi_sfr`. > > > > While there, also annotate array `data` with __counted_by() to prepare > > for the coming implementation by GCC and Clang of the __counted_by > > attribute. Flexible array members annotated with __counted_by can have > > their accesses bounds-checked at run-time via CONFIG_UBSAN_BOUNDS (for > > array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family > > functions). > > > > This results in no differences in binary output. > > Thanks for checking! Sorry for shameless plug, a quick question: has any compiler implemented support for counted_by() at this point?
> Sorry for shameless plug, a quick question: has any compiler implemented > support for counted_by() at this point? > Not yet. And at least for GCC, it's expected to be released in v15. Thanks -- Gustavo
On (24/01/09 07:17), Gustavo A. R. Silva wrote: > > > Sorry for shameless plug, a quick question: has any compiler implemented > > support for counted_by() at this point? > > > > Not yet. And at least for GCC, it's expected to be released in v15. I see. Thank you. I got confused by include/linux/compiler_attributes.h comment, as I'm on clang-18 currently, seems that we need to bump min compilers version. Oh, and clang link 404-s on me. I'll send a quick patch, I guess.
On 1/9/24 07:28, Sergey Senozhatsky wrote: > On (24/01/09 07:17), Gustavo A. R. Silva wrote: >> >>> Sorry for shameless plug, a quick question: has any compiler implemented >>> support for counted_by() at this point? >>> >> >> Not yet. And at least for GCC, it's expected to be released in v15. > > I see. Thank you. > > I got confused by include/linux/compiler_attributes.h comment, as I'm on > clang-18 currently, seems that we need to bump min compilers version. Ah yes, compiler devs have been running into some issues, and they had to postpone the release of the attribute. > Oh, and clang link 404-s on me. I'll send a quick patch, I guess. > You're right, ick! -- Gustavo
diff --git a/drivers/media/platform/qcom/venus/hfi_cmds.h b/drivers/media/platform/qcom/venus/hfi_cmds.h index dd9c5066442d..20acd412ee7b 100644 --- a/drivers/media/platform/qcom/venus/hfi_cmds.h +++ b/drivers/media/platform/qcom/venus/hfi_cmds.h @@ -242,7 +242,7 @@ struct hfi_session_parse_sequence_header_pkt { struct hfi_sfr { u32 buf_size; - u8 data[1]; + u8 data[] __counted_by(buf_size); }; struct hfi_sys_test_ssr_pkt {