re: NFC: Fragment LLCP I frames

re: NFC: Fragment LLCP I frames

[Dan Carpenter]

Hi Samuel,

I had some questions about the patch e65b0f46edfd: "NFC: Fragment LLCP I
frames" from Mar 5, 2012.

net/nfc/llcp/commands.c
+       while (remaining_len > 0) {
+
+               frag_len = min_t(u16, local->remote_miu, remaining_len);
                                 ^^^

This should be a cast to size_t.  Otherwise for a large value of
remaining_len we'd loop until we hit an allocation failure with
pdu = llcp_allocate_pdu();

 
-       sk = &sock->sk;
-       lock_sock(sk);
+               pr_debug("Fragment %zd bytes remaining %zd",
+                        frag_len, remaining_len);
 
-       nfc_llcp_queue_i_frames(sock);
+               pdu = llcp_allocate_pdu(sock, LLCP_PDU_I,
+                                       frag_len + LLCP_SEQUENCE_SIZE);
+               if (pdu == NULL)
+                       return -ENOMEM;
+
+               skb_put(pdu, LLCP_SEQUENCE_SIZE);
+
+               memcpy(skb_put(pdu, frag_len), msg_ptr, frag_len);
+
+               skb_queue_head(&sock->tx_queue, pdu);
+
+               lock_sock(sk);
+
+               nfc_llcp_queue_i_frames(sock);
+
+               release_sock(sk);
+
+               remaining_len -= frag_len;
+               msg_ptr += len;

Shouldn't this be msg_ptr += frag_len?

+       }

regards,
dan carpenter

Re: NFC: Fragment LLCP I frames

[Samuel Ortiz]

Hi Dan,

On Wed, Mar 07, 2012 at 12:51:42PM +0300, Dan Carpenter wrote:
> Hi Samuel,
> 
> I had some questions about the patch e65b0f46edfd: "NFC: Fragment LLCP I
> frames" from Mar 5, 2012.
> 
> net/nfc/llcp/commands.c
> +       while (remaining_len > 0) {
> +
> +               frag_len = min_t(u16, local->remote_miu, remaining_len);
>                                  ^^^
> 
> This should be a cast to size_t.  Otherwise for a large value of
> remaining_len we'd loop until we hit an allocation failure with
> pdu = llcp_allocate_pdu();
Right.


> +               pdu = llcp_allocate_pdu(sock, LLCP_PDU_I,
> +                                       frag_len + LLCP_SEQUENCE_SIZE);
> +               if (pdu == NULL)
> +                       return -ENOMEM;
> +
> +               skb_put(pdu, LLCP_SEQUENCE_SIZE);
> +
> +               memcpy(skb_put(pdu, frag_len), msg_ptr, frag_len);
> +
> +               skb_queue_head(&sock->tx_queue, pdu);
> +
> +               lock_sock(sk);
> +
> +               nfc_llcp_queue_i_frames(sock);
> +
> +               release_sock(sk);
> +
> +               remaining_len -= frag_len;
> +               msg_ptr += len;
> 
> Shouldn't this be msg_ptr += frag_len?
Right as well.
I'll send an patch to John for fixing that. Do you mind me adding a:
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

to it ?

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

Re: NFC: Fragment LLCP I frames

[Dan Carpenter]

[-- Attachment #1: Type: text/plain, Size: 324 bytes --]

On Thu, Mar 08, 2012 at 03:49:52AM +0100, Samuel Ortiz wrote:
> > Shouldn't this be msg_ptr += frag_len?
> Right as well.
> I'll send an patch to John for fixing that. Do you mind me adding a:
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> to it ?

That would be great.

regards,
dan carpenter



[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]