On 12/6/2024 11:13 PM, Dheeraj Reddy Jonnalagadda wrote: > Add a bounds check to ath12k_mac_vdev_create() to prevent an out-of-bounds > read in the vif->link_conf array. The function uses link_id, derived from > arvif->link_id, to index the array. When link_id equals 15, the index > exceeds the bounds of the array, which contains only 15 elements. > > This issue occurs in the following code branch: > > if (arvif->link_id == ATH12K_DEFAULT_SCAN_LINK && vif->valid_links) > link_id = ffs(vif->valid_links) - 1; > else > link_id = arvif->link_id; > > When the first condition in the if statement is true and the second > condition is false, it implies that arvif->link_id equals 15 and > the else branch is taken, where link_id is set to 15, causing an > out-of-bounds access when vif->link_conf array is read using link_id > as index. > > Add a check to ensure that link_id does not exceed the valid range of the > vif->link_conf array. Log a warning and return -EINVAL if the check fails > to prevent undefined behavior. > > Changelog: > > v2: > - Updated the commit message as per the reviewer's suggestions > - Clarified the description of the bug in the commit message > - Added Fixes and Closes tags with relevant information As Kalle already mentioned, the changelog should come "after the cut" Please refer to: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#the-canonical-patch-format Namely: One good use for the additional comments after the --- marker is for a diffstat... Other comments relevant only to the moment or the maintainer, not suitable for the permanent changelog, should also go here. A good example of such comments might be patch changelogs which describe what has changed between the v1 and v2 version of the patch. > > Fixes: 90570ba4610 ("wifi: ath12k: do not return invalid link id for scan link") > Closes: https://scan7.scan.coverity.com/#/project-view/52337/11354?selectedIssue=1602214 there should not be a blank line here Also I just joined the Coverity "linux" project. I have access to the dashboard, but don't see that particular CID. Since you've been a member for a few months, is there something special I need to do to see that CID? Or is this CID in a project other than "linux"? I ask because I'm looking at A CID in the latest snapshot of "linux" and the URL is: https://scan5.scan.coverity.com/#/project-view/63541/10063?selectedIssue=1636666 So I'm guessing your CID is from a different project? > > Signed-off-by: Dheeraj Reddy Jonnalagadda <dheeraj.linuxdev@xxxxxxxxx> > --- to reiterate, the changelog goes here, with the latest version described first. > drivers/net/wireless/ath/ath12k/mac.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c > index 129607ac6c1a..c19b10e66f4a 100644 > --- a/drivers/net/wireless/ath/ath12k/mac.c > +++ b/drivers/net/wireless/ath/ath12k/mac.c > @@ -7725,6 +7725,12 @@ int ath12k_mac_vdev_create(struct ath12k *ar, struct ath12k_link_vif *arvif) > else > link_id = arvif->link_id; > > + if (link_id >= ARRAY_SIZE(vif->link_conf)) { > + ath12k_warn(ar->ab, "link_id %u exceeds max valid links for vif %pM\n", > + link_id, vif->addr); > + return -EINVAL; > + } > + > link_conf = wiphy_dereference(hw->wiphy, vif->link_conf[link_id]); > if (!link_conf) { > ath12k_warn(ar->ab, "unable to access bss link conf in vdev create for vif %pM link %u\n", Note that I don't need you to send a new patch if Kalle ACKs the current one; I can fixup the patch in the "pending" branch. But I would like to make sure I can see the CID in Coverity, so please let me know if I'm subscribed to the correct project. /jeff
