| Message ID | 20230824075121.121144-1-dmantipov@yandex.ru |
|---|---|
| State | New |
| Headers | show |
| Series | [1/5] wifi: ath11k: drop redundant check in ath11k_dp_rx_mon_dest_process() | expand |
On 8/24/2023 12:50 AM, Dmitry Antipov wrote: > In 'ath11k_dp_rx_mon_dest_process()', 'mon_dst_srng' points to > a member of 'srng_list', which is a fixed-size array inside > 'struct ath11k_hal'. This way, if 'ring_id' is valid (i. e. > between 0 and HAL_SRNG_RING_ID_MAX - 1 inclusive), 'mon_dst_srng' > can't be NULL and so relevant check may be dropped. > > Found by Linux Verification Center (linuxtesting.org) with SVACE. > > Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru> Acked-by: Jeff Johnson <quic_jjohnson@quicinc.com> > --- > drivers/net/wireless/ath/ath11k/dp_rx.c | 7 ------- > 1 file changed, 7 deletions(-) > > diff --git a/drivers/net/wireless/ath/ath11k/dp_rx.c b/drivers/net/wireless/ath/ath11k/dp_rx.c > index 1e488eed282b..3f315547878a 100644 > --- a/drivers/net/wireless/ath/ath11k/dp_rx.c > +++ b/drivers/net/wireless/ath/ath11k/dp_rx.c > @@ -5097,13 +5097,6 @@ static void ath11k_dp_rx_mon_dest_process(struct ath11k *ar, int mac_id, > > mon_dst_srng = &ar->ab->hal.srng_list[ring_id]; > > - if (!mon_dst_srng) { > - ath11k_warn(ar->ab, > - "HAL Monitor Destination Ring Init Failed -- %p", > - mon_dst_srng); > - return; > - } > - > spin_lock_bh(&pmon->mon_lock); > > ath11k_hal_srng_access_begin(ar->ab, mon_dst_srng);
On 8/24/2023 12:50 AM, Dmitry Antipov wrote: > Since 'ath11k_spectral_pull_summary()' and 'ath11k_spectral_pull_search()' > always returns 0, both of them may be converted to 'void' and > 'ath11k_spectral_process_fft()' may be simplified accordingly. > > Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru> Acked-by: Jeff Johnson <quic_jjohnson@quicinc.com> > --- > drivers/net/wireless/ath/ath11k/spectral.c | 24 ++++++++-------------- > 1 file changed, 8 insertions(+), 16 deletions(-) > > diff --git a/drivers/net/wireless/ath/ath11k/spectral.c b/drivers/net/wireless/ath/ath11k/spectral.c > index 705868198df4..97eb2a457685 100644 > --- a/drivers/net/wireless/ath/ath11k/spectral.c > +++ b/drivers/net/wireless/ath/ath11k/spectral.c > @@ -470,10 +470,10 @@ static const struct file_operations fops_scan_bins = { > .llseek = default_llseek, > }; > > -static int ath11k_spectral_pull_summary(struct ath11k *ar, > - struct wmi_dma_buf_release_meta_data *meta, > - struct spectral_summary_fft_report *summary, > - struct ath11k_spectral_summary_report *report) > +static void ath11k_spectral_pull_summary(struct ath11k *ar, > + struct wmi_dma_buf_release_meta_data *meta, > + struct spectral_summary_fft_report *summary, > + struct ath11k_spectral_summary_report *report) > { > report->timestamp = __le32_to_cpu(summary->timestamp); > report->agc_total_gain = FIELD_GET(SPECTRAL_SUMMARY_INFO0_AGC_TOTAL_GAIN, > @@ -500,13 +500,11 @@ static int ath11k_spectral_pull_summary(struct ath11k *ar, > __le32_to_cpu(summary->info2)); > > memcpy(&report->meta, meta, sizeof(*meta)); > - > - return 0; > } > > -static int ath11k_spectral_pull_search(struct ath11k *ar, > - struct spectral_search_fft_report *search, > - struct ath11k_spectral_search_report *report) > +static void ath11k_spectral_pull_search(struct ath11k *ar, > + struct spectral_search_fft_report *search, > + struct ath11k_spectral_search_report *report) > { > report->timestamp = __le32_to_cpu(search->timestamp); > report->detector_id = FIELD_GET(SPECTRAL_FFT_REPORT_INFO0_DETECTOR_ID, > @@ -531,8 +529,6 @@ static int ath11k_spectral_pull_search(struct ath11k *ar, > __le32_to_cpu(search->info2)); > report->rel_pwr_db = FIELD_GET(SPECTRAL_FFT_REPORT_INFO2_REL_PWR_DB, > __le32_to_cpu(search->info2)); > - > - return 0; > } > > static u8 ath11k_spectral_get_max_exp(s8 max_index, u8 max_magnitude, > @@ -629,11 +625,7 @@ int ath11k_spectral_process_fft(struct ath11k *ar, > return ret; > } > > - ret = ath11k_spectral_pull_search(ar, data, &search); > - if (ret) { > - ath11k_warn(ab, "failed to pull search report %d\n", ret); > - return ret; > - } > + ath11k_spectral_pull_search(ar, data, &search); > > chan_width_mhz = summary->meta.ch_width; >
On 8/24/2023 12:50 AM, Dmitry Antipov wrote: > Use 'kstrtoul_from_user()' in 'ath11k_write_file_spectral_count()' > and 'ath11k_write_file_spectral_bins()' > > Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru> Acked-by: Jeff Johnson <quic_jjohnson@quicinc.com> > --- > drivers/net/wireless/ath/ath11k/spectral.c | 26 +++++++--------------- > 1 file changed, 8 insertions(+), 18 deletions(-) > > diff --git a/drivers/net/wireless/ath/ath11k/spectral.c b/drivers/net/wireless/ath/ath11k/spectral.c > index 97eb2a457685..b5530e484507 100644 > --- a/drivers/net/wireless/ath/ath11k/spectral.c > +++ b/drivers/net/wireless/ath/ath11k/spectral.c > @@ -382,16 +382,11 @@ static ssize_t ath11k_write_file_spectral_count(struct file *file, > { > struct ath11k *ar = file->private_data; > unsigned long val; > - char buf[32]; > - ssize_t len; > - > - len = min(count, sizeof(buf) - 1); > - if (copy_from_user(buf, user_buf, len)) > - return -EFAULT; > + ssize_t ret; > > - buf[len] = '\0'; > - if (kstrtoul(buf, 0, &val)) > - return -EINVAL; > + ret = kstrtoul_from_user(user_buf, count, 0, &val); > + if (ret) > + return ret; > > if (val > ATH11K_SPECTRAL_SCAN_COUNT_MAX) > return -EINVAL; > @@ -437,16 +432,11 @@ static ssize_t ath11k_write_file_spectral_bins(struct file *file, > { > struct ath11k *ar = file->private_data; > unsigned long val; > - char buf[32]; > - ssize_t len; > - > - len = min(count, sizeof(buf) - 1); > - if (copy_from_user(buf, user_buf, len)) > - return -EFAULT; > + ssize_t ret; > > - buf[len] = '\0'; > - if (kstrtoul(buf, 0, &val)) > - return -EINVAL; > + ret = kstrtoul_from_user(user_buf, count, 0, &val); > + if (ret) > + return ret; > > if (val < ATH11K_SPECTRAL_MIN_BINS || > val > ar->ab->hw_params.spectral.max_fft_bins)
On 8/24/2023 12:50 AM, Dmitry Antipov wrote: > Remove set but otherwise unused 'wlan_init_status' and > 'wmi_ready' members of 'struct ath11k_base', adjust > 'ath11k_wmi_tlv_rdy_parse()' accordingly. > > Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru> Acked-by: Jeff Johnson <quic_jjohnson@quicinc.com> > --- > drivers/net/wireless/ath/ath11k/core.h | 2 -- > drivers/net/wireless/ath/ath11k/wmi.c | 2 -- > 2 files changed, 4 deletions(-) > > diff --git a/drivers/net/wireless/ath/ath11k/core.h b/drivers/net/wireless/ath/ath11k/core.h > index b04447762483..650972f9d146 100644 > --- a/drivers/net/wireless/ath/ath11k/core.h > +++ b/drivers/net/wireless/ath/ath11k/core.h > @@ -901,8 +901,6 @@ struct ath11k_base { > struct list_head peers; > wait_queue_head_t peer_mapping_wq; > u8 mac_addr[ETH_ALEN]; > - bool wmi_ready; > - u32 wlan_init_status; > int irq_num[ATH11K_IRQ_NUM_MAX]; > struct ath11k_ext_irq_grp ext_irq_grp[ATH11K_EXT_IRQ_GRP_NUM_MAX]; > struct ath11k_targ_cap target_caps; > diff --git a/drivers/net/wireless/ath/ath11k/wmi.c b/drivers/net/wireless/ath/ath11k/wmi.c > index 23ad6825e5be..a5cf97368a14 100644 > --- a/drivers/net/wireless/ath/ath11k/wmi.c > +++ b/drivers/net/wireless/ath/ath11k/wmi.c > @@ -7222,14 +7222,12 @@ static int ath11k_wmi_tlv_rdy_parse(struct ath11k_base *ab, u16 tag, u16 len, > memset(&fixed_param, 0, sizeof(fixed_param)); > memcpy(&fixed_param, (struct wmi_ready_event *)ptr, > min_t(u16, sizeof(fixed_param), len)); > - ab->wlan_init_status = fixed_param.ready_event_min.status; > rdy_parse->num_extra_mac_addr = > fixed_param.ready_event_min.num_extra_mac_addr; > > ether_addr_copy(ab->mac_addr, > fixed_param.ready_event_min.mac_addr.addr); > ab->pktlog_defs_checksum = fixed_param.pktlog_defs_checksum; > - ab->wmi_ready = true; > break; > case WMI_TAG_ARRAY_FIXED_STRUCT: > addr_list = (struct wmi_mac_addr *)ptr;
Dmitry Antipov <dmantipov@yandex.ru> wrote: > In 'ath11k_dp_rx_mon_dest_process()', 'mon_dst_srng' points to > a member of 'srng_list', which is a fixed-size array inside > 'struct ath11k_hal'. This way, if 'ring_id' is valid (i. e. > between 0 and HAL_SRNG_RING_ID_MAX - 1 inclusive), 'mon_dst_srng' > can't be NULL and so relevant check may be dropped. > > Found by Linux Verification Center (linuxtesting.org) with SVACE. > > Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru> > Acked-by: Jeff Johnson <quic_jjohnson@quicinc.com> > Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com> 2 patches applied to ath-next branch of ath.git, thanks. 82ae3f463538 wifi: ath11k: drop redundant check in ath11k_dp_rx_mon_dest_process() 9066794113c4 wifi: ath11k: remove unused members of 'struct ath11k_base'
diff --git a/drivers/net/wireless/ath/ath11k/dp_rx.c b/drivers/net/wireless/ath/ath11k/dp_rx.c index 1e488eed282b..3f315547878a 100644 --- a/drivers/net/wireless/ath/ath11k/dp_rx.c +++ b/drivers/net/wireless/ath/ath11k/dp_rx.c @@ -5097,13 +5097,6 @@ static void ath11k_dp_rx_mon_dest_process(struct ath11k *ar, int mac_id, mon_dst_srng = &ar->ab->hal.srng_list[ring_id]; - if (!mon_dst_srng) { - ath11k_warn(ar->ab, - "HAL Monitor Destination Ring Init Failed -- %p", - mon_dst_srng); - return; - } - spin_lock_bh(&pmon->mon_lock); ath11k_hal_srng_access_begin(ar->ab, mon_dst_srng);
In 'ath11k_dp_rx_mon_dest_process()', 'mon_dst_srng' points to a member of 'srng_list', which is a fixed-size array inside 'struct ath11k_hal'. This way, if 'ring_id' is valid (i. e. between 0 and HAL_SRNG_RING_ID_MAX - 1 inclusive), 'mon_dst_srng' can't be NULL and so relevant check may be dropped. Found by Linux Verification Center (linuxtesting.org) with SVACE. Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru> --- drivers/net/wireless/ath/ath11k/dp_rx.c | 7 ------- 1 file changed, 7 deletions(-)