Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:2 as permitted sender) client-ip=2620:137:e000::3:2;
Message-ID: <e5f86461-fc67-d109-229c-b9d3fefd263a@intel.com>
Date:   Fri, 22 Sep 2023 11:04:04 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Betterbird/102.13.0
Subject: Re: [PATCH] libnvdimm: Annotate struct nd_region with __counted_by
To:     Kees Cook <keescook@chromium.org>,
        Dan Williams <dan.j.williams@intel.com>
CC:     Vishal Verma <vishal.l.verma@intel.com>,
        Ira Weiny <ira.weiny@intel.com>, <nvdimm@lists.linux.dev>,
        Nathan Chancellor <nathan@kernel.org>,
        "Nick Desaulniers" <ndesaulniers@google.com>,
        Tom Rix <trix@redhat.com>, <linux-kernel@vger.kernel.org>,
        <llvm@lists.linux.dev>, <linux-hardening@vger.kernel.org>
References: <20230922175238.work.116-kees@kernel.org>
Content-Language: en-US
From:   Dave Jiang <dave.jiang@intel.com>
In-Reply-To: <20230922175238.work.116-kees@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?TnVRTVBFdEhCa21pcUtEQVJsQWI3SUN1M1pyblltNDYrZURYcmwraHZ3YlFQ?=
 =?utf-8?B?THlvemxzcms5cWNwalF3ZWxHQWE2QjZ1VGl0TXlQMVJJelpQL1FHTnlaellq?=
 =?utf-8?B?VDBvL0tiMTBFSW1qOGhXd1d3UWk3S21mKzl3bnNGWmVHb2Ric002S3pGYkts?=
 =?utf-8?B?YVdrd1h3cS8wa0psTnhhdmJIaDZkeFVidXBDOURKUWZJNDRJU3ltWlcrOFFH?=
 =?utf-8?B?S3FmamNxbWNwbnhvYWROZ1FhOEZLcmNKbHQ5OFZsSHVrSTRHbXgyVVFwWmhS?=
 =?utf-8?B?WXE0Ky95Y2VpdUU2a0JnaU1wSTl3Mk9oVFpUSyt3ai9USzVZU3MrUG1uaHNw?=
 =?utf-8?B?OE5KZWg3eG9ndHplK3htVEdTZ0hDYjNabnd0SVRqR2RNdjhDUEhKNDNhNmF5?=
 =?utf-8?B?THlPakNKTHJ4ZFMvWlhva0VQeU52bzVDNWUyTm1CZTNjbllpWHM2bDNyazdy?=
 =?utf-8?B?eHBmYzB6VUo5RFBuSGk3Z0llQ2JCWlZ1M011em1oc2RMcDRVOUtpYXBvTk9W?=
 =?utf-8?B?dGZodGdpOGFiM2pFRjZLRjZjZmhTWTNla00zSUUra2VGZ0tPTEphVjNvQzVt?=
 =?utf-8?B?WjFyOXRobDRTbkZrUU5qczFkOE50WUpvalJxdjlHVnJsaGp6NlNCTWcvTW5l?=
 =?utf-8?B?VVQ1SnBRQndxcjVhK0NVM1JOL3gwSlEvdlBUeEdTU0tXaDArTnBPSlVKYnNG?=
 =?utf-8?B?VWFCbVBsZm00Z3dPbFBvRzkzaWNwMGpvNjlQVmRXOHFYUGNYRTFwSkpPRG5R?=
 =?utf-8?B?WHFoWVUwdnladUdQMUpuWGQzbFRkMjRVWGNja0Fva2I3V1ZtWXdoK3Q3REY3?=
 =?utf-8?B?cEthTy9jQlNSMkMwbHVxQWFYbyt3dDdoOE9YNmEvR2hJalo1S1FSVmdDQnMr?=
 =?utf-8?B?aGpZQnJqbC96MEtDWllmQ2thYW1lOS92VHMyMktncjdXdTJiaFhjVmdsam5v?=
 =?utf-8?B?SmdWRUpEZm83RG50UWxpZTlhb1Y1KzFsOWd4elFnYVJJUi9jN0JWOXpETjZo?=
 =?utf-8?B?Zk1VY2pjY3I1OEVkVW9XOGFqajhHK1duRzVBbWpSQndINUFGeHhtcVhIN2Fh?=
 =?utf-8?B?SUxhc3FwNDFMQXdrenVzdzFTTkZlL3o0dHo2cS9ZcVBtQmZYRENncUNnOHNo?=
 =?utf-8?B?b29EenBZQUdKeGdyaEMxZnlKb3FEZGxFb1JHKzBzbzlEayt4ZlAwVU9PSCsv?=
 =?utf-8?B?QTBPbTBYRFFNSzZoVXh4aVJWWTRXa1JHZUUwWU5oWVJTMVRacldTbU1TRzNh?=
 =?utf-8?B?NXl1bWVBSmp6U1BIVWliOG9QNnkwWWJKVktYTFprRW0wTW44Qko5NzgyYnBa?=
 =?utf-8?B?eHpEUXlUalNNZkk4eGlRRUZadzN4Q0ZTZjhVMFRTSUw3ckxvN2VVZ3IrU01D?=
 =?utf-8?B?ZkpuZlFRclpDWDlkdjdJYlRTU3RJQkdQOGdlVTBDWms2QnZzbXdxMXBOenlH?=
 =?utf-8?B?eGxrOCt2QktLRWxLTE1VeDFQM0krNXFVWXdoOS9sWGpIeTR3NWlNZTdKc0FM?=
 =?utf-8?B?cVNwT3QwbGxoMFhRKytGTjNZVDJpK1VtMlhhNmlPTHJxTmZuOE5mLzR6Wjc0?=
 =?utf-8?B?MVpjZzk5Wlc0dE13RngvbVgzckUweDFEMmJRTFZtV1RKbUNXMUlhZzBrNHI4?=
 =?utf-8?B?alJ3OTNBbGZqb0I0RlZlQkIvQzFwd3NPSUJDU2YxUzMrUGpqczVoK01rMUtr?=
 =?utf-8?B?ZWIydlJoU2ZTQXFXdkhSU0ZjbWN4b2dSbTdPaHI0T2JXOThJU2ttNzE0OW9I?=
 =?utf-8?B?Vjg1Vk9FZWZKOU1CZTJjbjlLYTRsYTVrMThLRUNwYUtnZ0hvNVJaTWpDZU9n?=
 =?utf-8?B?WVRaNTdpckgyUkViNEFNV1Y4ZU8wODF1STJrNHhLM0dWOEtDMWhuL0ZubmhK?=
 =?utf-8?B?b1REeThiS0VKaWZvWkNLZDExQVEzbm9GL3V6WEliczhYamtVM0xPR2pIamVE?=
 =?utf-8?B?OThGVm1pdE50eXh3MXNqOXZHd3c1bjIvWGRmR3M3aFI1TUtacDFIU29RUDVZ?=
 =?utf-8?B?b2o5Qzd3a2VRaUV0VkFDdS9SOUdEcHdTTlB0VUtWb25Sc3FhZHlXM1NOZzlK?=
 =?utf-8?B?RkJ3aGk0bUE3bzhjTlZ6Mit1UENyZGtjMWdxMFBwakVlbTBES2lLNnN5clRr?=
 =?utf-8?Q?dC3/FTZPXdWJqlOIWLKlllGrv?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 617dee4f-195d-456f-ab17-08dbbb965196
X-MS-Exchange-CrossTenant-AuthSource: PH7PR11MB5984.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 Sep 2023 18:04:09.5075
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 7ohOn2Axh0hShH0FLctfSvNUA+fcFEyv5MWKw67o8SeC1mfY/fQdTvAnCcq+FhkVGdWIm/dAGmuRAgMgnE2sDg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA0PR11MB4544
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-2.3 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED
	autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on agentk.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 (agentk.vger.email [0.0.0.0]); Fri, 22 Sep 2023 11:04:22 -0700 (PDT)


On 9/22/23 10:52, Kees Cook wrote:
> 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 checking via CONFIG_UBSAN_BOUNDS
> (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family
> functions).
> 
> As found with Coccinelle[1], add __counted_by for struct nd_region.
> Additionally, since the element count member must be set before accessing
> the annotated flexible array member, move its initialization earlier.
> 
> [1] https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci
> 
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Vishal Verma <vishal.l.verma@intel.com>
> Cc: Dave Jiang <dave.jiang@intel.com>
> Cc: Ira Weiny <ira.weiny@intel.com>
> Cc: nvdimm@lists.linux.dev
> Signed-off-by: Kees Cook <keescook@chromium.org>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> ---
>  drivers/nvdimm/nd.h          | 2 +-
>  drivers/nvdimm/region_devs.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/nvdimm/nd.h b/drivers/nvdimm/nd.h
> index e8b9d27dbb3c..ae2078eb6a62 100644
> --- a/drivers/nvdimm/nd.h
> +++ b/drivers/nvdimm/nd.h
> @@ -422,7 +422,7 @@ struct nd_region {
>  	struct nd_interleave_set *nd_set;
>  	struct nd_percpu_lane __percpu *lane;
>  	int (*flush)(struct nd_region *nd_region, struct bio *bio);
> -	struct nd_mapping mapping[];
> +	struct nd_mapping mapping[] __counted_by(ndr_mappings);
>  };
>  
>  static inline bool nsl_validate_nlabel(struct nd_region *nd_region,
> diff --git a/drivers/nvdimm/region_devs.c b/drivers/nvdimm/region_devs.c
> index 0a81f87f6f6c..5be65fce85cf 100644
> --- a/drivers/nvdimm/region_devs.c
> +++ b/drivers/nvdimm/region_devs.c
> @@ -1028,6 +1028,7 @@ static struct nd_region *nd_region_create(struct nvdimm_bus *nvdimm_bus,
>  
>  	if (!nd_region)
>  		return NULL;
> +	nd_region->ndr_mappings = ndr_desc->num_mappings;
>  	/* CXL pre-assigns memregion ids before creating nvdimm regions */
>  	if (test_bit(ND_REGION_CXL, &ndr_desc->flags)) {
>  		nd_region->id = ndr_desc->memregion;
> @@ -1062,7 +1063,6 @@ static struct nd_region *nd_region_create(struct nvdimm_bus *nvdimm_bus,
>  
>  		get_device(&nvdimm->dev);
>  	}
> -	nd_region->ndr_mappings = ndr_desc->num_mappings;
>  	nd_region->provider_data = ndr_desc->provider_data;
>  	nd_region->nd_set = ndr_desc->nd_set;
>  	nd_region->num_lanes = ndr_desc->num_lanes;