* [smatch stuff] question about iwlagn_rx_calib_result()
@ 2011-12-07 8:59 Dan Carpenter
2011-12-07 9:07 ` Johannes Berg
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2011-12-07 8:59 UTC (permalink / raw)
To: Wey-Yi Guy; +Cc: Intel Linux Wireless, linux-wireless
Smatch complains about iwlagn_rx_calib_result() it would be bad for
"len" to be negative.
drivers/net/wireless/iwlwifi/iwl-ucode.c
299 int iwlagn_rx_calib_result(struct iwl_priv *priv,
300 struct iwl_rx_mem_buffer *rxb,
301 struct iwl_device_cmd *cmd)
302 {
303 struct iwl_rx_packet *pkt = rxb_addr(rxb);
304 struct iwl_calib_hdr *hdr = (struct iwl_calib_hdr *)pkt->u.raw;
305 int len = le32_to_cpu(pkt->len_n_flags) & FH_RSCSR_FRAME_SIZE_MSK;
306
307 /* reduce the size of the length field itself */
308 len -= 4;
^^^^^^^^
Where does this 4 come from? I've tried to determine what the minimum
size of "le32_to_cpu(pkt->len_n_flags) & FH_RSCSR_FRAME_SIZE_MSK" is but
I got lost. Can it ever be less than 4?
309
310 if (iwl_calib_set(priv, hdr, len))
311 IWL_ERR(priv, "Failed to record calibration data %d\n",
312 hdr->op_code);
313
314 return 0;
315 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [smatch stuff] question about iwlagn_rx_calib_result()
2011-12-07 8:59 [smatch stuff] question about iwlagn_rx_calib_result() Dan Carpenter
@ 2011-12-07 9:07 ` Johannes Berg
2011-12-07 9:10 ` Johannes Berg
0 siblings, 1 reply; 3+ messages in thread
From: Johannes Berg @ 2011-12-07 9:07 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Wey-Yi Guy, Intel Linux Wireless, linux-wireless
On Wed, 2011-12-07 at 11:59 +0300, Dan Carpenter wrote:
> Smatch complains about iwlagn_rx_calib_result() it would be bad for
> "len" to be negative.
>
> drivers/net/wireless/iwlwifi/iwl-ucode.c
>
> 299 int iwlagn_rx_calib_result(struct iwl_priv *priv,
> 300 struct iwl_rx_mem_buffer *rxb,
> 301 struct iwl_device_cmd *cmd)
> 302 {
> 303 struct iwl_rx_packet *pkt = rxb_addr(rxb);
> 304 struct iwl_calib_hdr *hdr = (struct iwl_calib_hdr *)pkt->u.raw;
> 305 int len = le32_to_cpu(pkt->len_n_flags) & FH_RSCSR_FRAME_SIZE_MSK;
> 306
> 307 /* reduce the size of the length field itself */
> 308 len -= 4;
> ^^^^^^^^
> Where does this 4 come from? I've tried to determine what the minimum
> size of "le32_to_cpu(pkt->len_n_flags) & FH_RSCSR_FRAME_SIZE_MSK" is but
> I got lost. Can it ever be less than 4?
4 is sizeof(struct iwl_cmd_header) I think.
If the frame size ends up <= 4, that would be a major uCode/device bug
since any frame has to start with len_n_flags & the header, of which the
len_n_flags isn't included (see struct iwl_rx_packet).
johannes
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [smatch stuff] question about iwlagn_rx_calib_result()
2011-12-07 9:07 ` Johannes Berg
@ 2011-12-07 9:10 ` Johannes Berg
0 siblings, 0 replies; 3+ messages in thread
From: Johannes Berg @ 2011-12-07 9:10 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Wey-Yi Guy, Intel Linux Wireless, linux-wireless
On Wed, 2011-12-07 at 10:07 +0100, Johannes Berg wrote:
> On Wed, 2011-12-07 at 11:59 +0300, Dan Carpenter wrote:
> > Smatch complains about iwlagn_rx_calib_result() it would be bad for
> > "len" to be negative.
> >
> > drivers/net/wireless/iwlwifi/iwl-ucode.c
> >
> > 299 int iwlagn_rx_calib_result(struct iwl_priv *priv,
> > 300 struct iwl_rx_mem_buffer *rxb,
> > 301 struct iwl_device_cmd *cmd)
> > 302 {
> > 303 struct iwl_rx_packet *pkt = rxb_addr(rxb);
> > 304 struct iwl_calib_hdr *hdr = (struct iwl_calib_hdr *)pkt->u.raw;
> > 305 int len = le32_to_cpu(pkt->len_n_flags) & FH_RSCSR_FRAME_SIZE_MSK;
> > 306
> > 307 /* reduce the size of the length field itself */
> > 308 len -= 4;
> > ^^^^^^^^
> > Where does this 4 come from? I've tried to determine what the minimum
> > size of "le32_to_cpu(pkt->len_n_flags) & FH_RSCSR_FRAME_SIZE_MSK" is but
> > I got lost. Can it ever be less than 4?
>
> 4 is sizeof(struct iwl_cmd_header) I think.
>
> If the frame size ends up <= 4, that would be a major uCode/device bug
> since any frame has to start with len_n_flags & the header, of which the
> len_n_flags isn't included (see struct iwl_rx_packet).
Yeah, see iwl_rx_handle(). It adds sizeof(u32) for the rate_n_flags
field ("status word"). I suppose there could be some value in checking
that the length is at least 4, but I guess it should be in
iwl_rx_handle() and then smatch would still complain about
rx_calib_result().
johannes
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2011-12-07 9:10 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-12-07 8:59 [smatch stuff] question about iwlagn_rx_calib_result() Dan Carpenter
2011-12-07 9:07 ` Johannes Berg
2011-12-07 9:10 ` Johannes Berg
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).