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
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
