> Should a check be added here that desc_buf[QUERY_DESC_LENGTH_OFFSET] > <= QUERY_DESC_MAX_SIZE to protect against firmware bugs? Since > QUERY_DESC_MAX_SIZE == 255, adding BUILD_BUG_ON(QUERY_DESC_SIZE != > 255) > and a comment may be sufficient. Sorry - my bad. In the previous review you mentioned that the patch looks good to you, hence the "Review by". Regarding your comment, we can do that, although I don't think we should cover for those FW basic bugs. Please let me know that you prefer. Regards Arthur > -----Original Message----- > From: Bart Van Assche <bvanassche@xxxxxxx> > Sent: Monday, December 12, 2022 2:19 AM > To: Arthur Simchaev <Arthur.Simchaev@xxxxxxx>; > martin.petersen@xxxxxxxxxx > Cc: beanhuo@xxxxxxxxxx; linux-scsi@xxxxxxxxxxxxxxx; linux- > kernel@xxxxxxxxxxxxxxx > Subject: Re: [PATCH v5 4/4] ufs: core: Remove ufshcd_map_desc_id_to_length > function > > CAUTION: This email originated from outside of Western Digital. Do not click > on links or open attachments unless you recognize the sender and know that > the content is safe. > > > On 12/11/22 05:05, Arthur Simchaev wrote: > > There shouldn't be any restriction of the descriptor size > > (not the descriptor id for that matter) up to QUERY_DESC_MAX_SIZE. > > According to the spec, the caller can use any descriptor size, > > and it is up to the device to return the actual size. > > Therefore there shouldn't be any sizes hardcoded in the kernel, > > nor any need to cache it, hence ufshcd_map_desc_id_to_length function is > > redundant. Always read the descriptors with QUERY_DESC_MAX_SIZE size. > > > > Reviewed-by: Bart Van Assche <bvanassche@xxxxxxx> > > I have not yet replied with "Reviewed-by" to this patch so you are not > yet allowed to add my Reviewed-by tag to this patch. > > > + /* Update descriptor length */ > > + buff_len = desc_buf[QUERY_DESC_LENGTH_OFFSET]; > > Should a check be added here that desc_buf[QUERY_DESC_LENGTH_OFFSET] > <= QUERY_DESC_MAX_SIZE to protect against firmware bugs? Since > QUERY_DESC_MAX_SIZE == 255, adding BUILD_BUG_ON(QUERY_DESC_SIZE != > 255) > and a comment may be sufficient. > > Thanks, > > Bart.
