[Qemu-devel] [PATCH] slirp/tftp.c: fix mode field

[Qemu-devel] [PATCH] slirp/tftp.c: fix mode field

[Sergei Gavrikov]

Hi,

According to RFC 1350 http://www.ietf.org/rfc/rfc1350.txt [Page 5]:

    The mode field contains the string "netascii", "octet", or "mail"
    (or any combination of upper and lower case, such as "NETASCII",
    NetAscii", etc.)

Unfortunately, current implementation of internal TFTP server breaks the
requests with the mode fields like "OCTET\0". For example, the RedBoot's
TFTP client sends the same (in upper case). So, it is not possible to
get internal TFTP working with RedBoot loader. If you do not have doubts
about STRCASECMP(3), a patch is provided.

Thanks,
Sergei

Signed-off-by: Sergei Gavrikov <sergei.gavrikov@gmail.com>
---
 slirp/tftp.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/slirp/tftp.c b/slirp/tftp.c
index 55e4692..6ad1da0 100644
--- a/slirp/tftp.c
+++ b/slirp/tftp.c
@@ -311,7 +311,7 @@ static void tftp_handle_rrq(Slirp *slirp, struct tftp_t *tp, int pktlen)
     return;
   }
 
-  if (memcmp(&tp->x.tp_buf[k], "octet\0", 6) != 0) {
+  if (strcasecmp(&tp->x.tp_buf[k], "octet") != 0) {
       tftp_send_error(spt, 4, "Unsupported transfer mode", tp);
       return;
   }

[Qemu-devel] Re: [PATCH] slirp/tftp.c: fix mode field

[Sergei Gavrikov]

On Mon, 10 Jan 2011, Sergei Gavrikov wrote:
> According to RFC 1350 http://www.ietf.org/rfc/rfc1350.txt [Page 5]:
> 
>     The mode field contains the string "netascii", "octet", or "mail"
>     (or any combination of upper and lower case, such as "NETASCII",
>     NetAscii", etc.)
> 
> Unfortunately, current implementation of internal TFTP server breaks the
> requests with the mode fields like "OCTET\0". For example, the RedBoot's
> TFTP client sends the same (in upper case). So, it is not possible to
> get internal TFTP working with RedBoot loader. If you do not have doubts
> about STRCASECMP(3), a patch is provided.

... 

> diff --git a/slirp/tftp.c b/slirp/tftp.c
> index 55e4692..6ad1da0 100644
> --- a/slirp/tftp.c
> +++ b/slirp/tftp.c
> @@ -311,7 +311,7 @@ static void tftp_handle_rrq(Slirp *slirp, struct tftp_t *tp, int pktlen)
>      return;
>    }
>  
> -  if (memcmp(&tp->x.tp_buf[k], "octet\0", 6) != 0) {
> +  if (strcasecmp(&tp->x.tp_buf[k], "octet") != 0) {

The above adds a warning, renewed (no warning).

Signed-off-by: Sergei Gavrikov <sergei.gavrikov@gmail.com>
---
 slirp/tftp.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/slirp/tftp.c b/slirp/tftp.c
index 55e4692..a455ad1 100644
--- a/slirp/tftp.c
+++ b/slirp/tftp.c
@@ -311,7 +311,7 @@ static void tftp_handle_rrq(Slirp *slirp, struct tftp_t *tp, int pktlen)
     return;
   }
 
-  if (memcmp(&tp->x.tp_buf[k], "octet\0", 6) != 0) {
+  if (strcasecmp((const char *)&tp->x.tp_buf[k], "octet") != 0) {
       tftp_send_error(spt, 4, "Unsupported transfer mode", tp);
       return;
   }

Re: [Qemu-devel] Re: [PATCH] slirp/tftp.c: fix mode field

[Stefan Hajnoczi]

On Wed, Jan 12, 2011 at 7:22 AM, Sergei Gavrikov
<sergei.gavrikov@gmail.com> wrote:
> diff --git a/slirp/tftp.c b/slirp/tftp.c
> index 55e4692..a455ad1 100644
> --- a/slirp/tftp.c
> +++ b/slirp/tftp.c
> @@ -311,7 +311,7 @@ static void tftp_handle_rrq(Slirp *slirp, struct tftp_t *tp, int pktlen)
>     return;
>   }
>
> -  if (memcmp(&tp->x.tp_buf[k], "octet\0", 6) != 0) {
> +  if (strcasecmp((const char *)&tp->x.tp_buf[k], "octet") != 0) {
>       tftp_send_error(spt, 4, "Unsupported transfer mode", tp);
>       return;
>   }

According to RFC 2349 the "tsize" option is also case-insensitive.
Want to include a fix for that in this patch?

Stefan

Re: [Qemu-devel] Re: [PATCH] slirp/tftp.c: fix mode field

[Sergei Gavrikov]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1763 bytes --]

On Wed, 12 Jan 2011, Stefan Hajnoczi wrote:

> On Wed, Jan 12, 2011 at 7:22 AM, Sergei Gavrikov
> <sergei.gavrikov@gmail.com> wrote:
> > diff --git a/slirp/tftp.c b/slirp/tftp.c
> > index 55e4692..a455ad1 100644
> > --- a/slirp/tftp.c
> > +++ b/slirp/tftp.c
> > @@ -311,7 +311,7 @@ static void tftp_handle_rrq(Slirp *slirp, struct tftp_t *tp, int pktlen)
> >     return;
> >   }
> >
> > -  if (memcmp(&tp->x.tp_buf[k], "octet\0", 6) != 0) {
> > +  if (strcasecmp((const char *)&tp->x.tp_buf[k], "octet") != 0) {
> >       tftp_send_error(spt, 4, "Unsupported transfer mode", tp);
> >       return;
> >   }
> 
> According to RFC 2349 the "tsize" option is also case-insensitive.
> Want to include a fix for that in this patch?

It was good to know. If you mean the below I "merge" that in one. Thank
you for review.

Sergei

Signed-off-by: Sergei Gavrikov <sergei.gavrikov@gmail.com>
Acked-by: Stefan Hajnoczi <stefanha@gmail.com>
----
 slirp/tftp.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/slirp/tftp.c b/slirp/tftp.c
index 55e4692..1821648 100644
--- a/slirp/tftp.c
+++ b/slirp/tftp.c
@@ -311,7 +311,7 @@ static void tftp_handle_rrq(Slirp *slirp, struct tftp_t *tp, int pktlen)
     return;
   }
 
-  if (memcmp(&tp->x.tp_buf[k], "octet\0", 6) != 0) {
+  if (strcasecmp((const char *)&tp->x.tp_buf[k], "octet") != 0) {
       tftp_send_error(spt, 4, "Unsupported transfer mode", tp);
       return;
   }
@@ -351,7 +351,7 @@ static void tftp_handle_rrq(Slirp *slirp, struct tftp_t *tp, int pktlen)
       value = (const char *)&tp->x.tp_buf[k];
       k += strlen(value) + 1;
 
-      if (strcmp(key, "tsize") == 0) {
+      if (strcasecmp(key, "tsize") == 0) {
 	  int tsize = atoi(value);
 	  struct stat stat_p;
 

Re: [Qemu-devel] Re: [PATCH] slirp/tftp.c: fix mode field

[Stefan Hajnoczi]

On Wed, Jan 12, 2011 at 10:22 AM, Sergei Gavrikov
<sergei.gavrikov@gmail.com> wrote:
> It was good to know. If you mean the below I "merge" that in one. Thank
> you for review.
>
> Sergei
>
> Signed-off-by: Sergei Gavrikov <sergei.gavrikov@gmail.com>
> Acked-by: Stefan Hajnoczi <stefanha@gmail.com>

Your new patch looks fine.  I prefer:
Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>

Stefan

Re: [Qemu-devel] Re: [PATCH] slirp/tftp.c: fix mode field

[Sergei Gavrikov]

On Wed, 12 Jan 2011, Stefan Hajnoczi wrote:

> On Wed, Jan 12, 2011 at 10:22 AM, Sergei Gavrikov
> <sergei.gavrikov@gmail.com> wrote:
> > It was good to know. If you mean the below I "merge" that in one. Thank
> > you for review.
> >
> > Sergei
> >
> > Signed-off-by: Sergei Gavrikov <sergei.gavrikov@gmail.com>
> > Acked-by: Stefan Hajnoczi <stefanha@gmail.com>
> 
> Your new patch looks fine.  I prefer:
> Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> 
> Stefan

Excuse, please, that 'Acked-by'. I'm get it. Thank you!

Sergei

[Qemu-devel] [PATCHv1] slirp: Use strcasecmp() to check tftp mode, tsize

[Sergei Gavrikov]

From: Sergei Gavrikov <sergei.gavrikov@gmail.com>

According to RFC 1350 (TFTP Revision 2) the mode field can contain any
combination of upper and lower case; also RFC 2349 propagates that the
transfer size option ("tsize") is case in-sensitive too.

Current implementation of embedded TFTP server missed that what does
mess some TFTP clients. Fixed by using STRCASECMP(3) in the required
places.

Signed-off-by: Sergei Gavrikov <sergei.gavrikov@gmail.com>
Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 slirp/tftp.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/slirp/tftp.c b/slirp/tftp.c
index 55e4692..1821648 100644
--- a/slirp/tftp.c
+++ b/slirp/tftp.c
@@ -311,7 +311,7 @@ static void tftp_handle_rrq(Slirp *slirp, struct tftp_t *tp, int pktlen)
     return;
   }
 
-  if (memcmp(&tp->x.tp_buf[k], "octet\0", 6) != 0) {
+  if (strcasecmp((const char *)&tp->x.tp_buf[k], "octet") != 0) {
       tftp_send_error(spt, 4, "Unsupported transfer mode", tp);
       return;
   }
@@ -351,7 +351,7 @@ static void tftp_handle_rrq(Slirp *slirp, struct tftp_t *tp, int pktlen)
       value = (const char *)&tp->x.tp_buf[k];
       k += strlen(value) + 1;
 
-      if (strcmp(key, "tsize") == 0) {
+      if (strcasecmp(key, "tsize") == 0) {
 	  int tsize = atoi(value);
 	  struct stat stat_p;
 

Re: [Qemu-devel] [PATCHv1] slirp: Use strcasecmp() to check tftp mode, tsize

[Edgar E. Iglesias]

On Wed, Jan 12, 2011 at 03:57:18PM +0200, Sergei Gavrikov wrote:
> From: Sergei Gavrikov <sergei.gavrikov@gmail.com>
> 
> According to RFC 1350 (TFTP Revision 2) the mode field can contain any
> combination of upper and lower case; also RFC 2349 propagates that the
> transfer size option ("tsize") is case in-sensitive too.
> 
> Current implementation of embedded TFTP server missed that what does
> mess some TFTP clients. Fixed by using STRCASECMP(3) in the required
> places.
> 
> Signed-off-by: Sergei Gavrikov <sergei.gavrikov@gmail.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>

I've applied this, thanks both of you.

Cheers


> ---
>  slirp/tftp.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/slirp/tftp.c b/slirp/tftp.c
> index 55e4692..1821648 100644
> --- a/slirp/tftp.c
> +++ b/slirp/tftp.c
> @@ -311,7 +311,7 @@ static void tftp_handle_rrq(Slirp *slirp, struct tftp_t *tp, int pktlen)
>      return;
>    }
>  
> -  if (memcmp(&tp->x.tp_buf[k], "octet\0", 6) != 0) {
> +  if (strcasecmp((const char *)&tp->x.tp_buf[k], "octet") != 0) {
>        tftp_send_error(spt, 4, "Unsupported transfer mode", tp);
>        return;
>    }
> @@ -351,7 +351,7 @@ static void tftp_handle_rrq(Slirp *slirp, struct tftp_t *tp, int pktlen)
>        value = (const char *)&tp->x.tp_buf[k];
>        k += strlen(value) + 1;
>  
> -      if (strcmp(key, "tsize") == 0) {
> +      if (strcasecmp(key, "tsize") == 0) {
>  	  int tsize = atoi(value);
>  	  struct stat stat_p;
>  
>