Re: [ovs-dev] [PATCH] nx-match: Fix use-after-free parsing matches.

2016-03-29 Thread Joe Stringer
On 23 March 2016 at 06:41, Ben Pfaff  wrote:
> On Mon, Mar 07, 2016 at 11:31:02AM -0800, Joe Stringer wrote:
>> Address pointed by header_ptr might be free'd due to realloc
>> happened in ofpbuf_put_hex(). Reported by valgrind in the test
>> 379: check TCP flags expression in OXM and NXM.
>>
>> Invalid write of size 4
>> nx_match_from_string_raw (nx-match.c:1510)
>> nx_match_from_string (nx-match.c:1538)
>> ofctl_parse_nxm__ (ovs-ofctl.c:3325)
>> ovs_cmdl_run_command (command-line.c:121)
>> main (ovs-ofctl.c:137)
>>
>> Address 0x7a2cc40 is 0 bytes inside a block of size 64 free'd
>> free (vg_replace_malloc.c:530)
>> ofpbuf_resize__ (ofpbuf.c:246)
>> ofpbuf_put (ofpbuf.c:386)
>> ofpbuf_put_hex (ofpbuf.c:414)
>> nx_match_from_string_raw (nx-match.c:1488)
>> nx_match_from_string (nx-match.c:1538)
>> ofctl_parse_nxm__ (ovs-ofctl.c:3325)
>>
>> Reported-by: William Tu 
>> Signed-off-by: Joe Stringer 
>
> Acked-by: Ben Pfaff 

Thanks, applied to master.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] nx-match: Fix use-after-free parsing matches.

2016-03-22 Thread Ben Pfaff
On Mon, Mar 07, 2016 at 11:31:02AM -0800, Joe Stringer wrote:
> Address pointed by header_ptr might be free'd due to realloc
> happened in ofpbuf_put_hex(). Reported by valgrind in the test
> 379: check TCP flags expression in OXM and NXM.
> 
> Invalid write of size 4
> nx_match_from_string_raw (nx-match.c:1510)
> nx_match_from_string (nx-match.c:1538)
> ofctl_parse_nxm__ (ovs-ofctl.c:3325)
> ovs_cmdl_run_command (command-line.c:121)
> main (ovs-ofctl.c:137)
> 
> Address 0x7a2cc40 is 0 bytes inside a block of size 64 free'd
> free (vg_replace_malloc.c:530)
> ofpbuf_resize__ (ofpbuf.c:246)
> ofpbuf_put (ofpbuf.c:386)
> ofpbuf_put_hex (ofpbuf.c:414)
> nx_match_from_string_raw (nx-match.c:1488)
> nx_match_from_string (nx-match.c:1538)
> ofctl_parse_nxm__ (ovs-ofctl.c:3325)
> 
> Reported-by: William Tu 
> Signed-off-by: Joe Stringer 

Acked-by: Ben Pfaff 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] nx-match: Fix use-after-free parsing matches.

2016-03-07 Thread William Tu
Hi Joe,

I've tested this patch (with modification to ofpbuf_put to force using
newly allocated address) and it works fine. Thanks!

Regards,
William

On Mon, Mar 7, 2016 at 11:31 AM, Joe Stringer  wrote:

> Address pointed by header_ptr might be free'd due to realloc
> happened in ofpbuf_put_hex(). Reported by valgrind in the test
> 379: check TCP flags expression in OXM and NXM.
>
> Invalid write of size 4
> nx_match_from_string_raw (nx-match.c:1510)
> nx_match_from_string (nx-match.c:1538)
> ofctl_parse_nxm__ (ovs-ofctl.c:3325)
> ovs_cmdl_run_command (command-line.c:121)
> main (ovs-ofctl.c:137)
>
> Address 0x7a2cc40 is 0 bytes inside a block of size 64 free'd
> free (vg_replace_malloc.c:530)
> ofpbuf_resize__ (ofpbuf.c:246)
> ofpbuf_put (ofpbuf.c:386)
> ofpbuf_put_hex (ofpbuf.c:414)
> nx_match_from_string_raw (nx-match.c:1488)
> nx_match_from_string (nx-match.c:1538)
> ofctl_parse_nxm__ (ovs-ofctl.c:3325)
>
> Reported-by: William Tu 
> Signed-off-by: Joe Stringer 
> ---
>  lib/nx-match.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/lib/nx-match.c b/lib/nx-match.c
> index 4999b1ac95d7..6fcc53ad07d1 100644
> --- a/lib/nx-match.c
> +++ b/lib/nx-match.c
> @@ -1467,7 +1467,6 @@ nx_match_from_string_raw(const char *s, struct
> ofpbuf *b)
>  const char *name;
>  uint64_t header;
>  ovs_be64 nw_header;
> -ovs_be64 *header_ptr;
>  int name_len;
>  size_t n;
>
> @@ -1484,7 +1483,7 @@ nx_match_from_string_raw(const char *s, struct
> ofpbuf *b)
>
>  s += name_len + 1;
>
> -header_ptr = ofpbuf_put_uninit(b, nxm_header_len(header));
> +b->header = ofpbuf_put_uninit(b, nxm_header_len(header));
>  s = ofpbuf_put_hex(b, s, &n);
>  if (n != nxm_field_bytes(header)) {
>  const struct mf_field *field = mf_from_oxm_header(header);
> @@ -1507,7 +1506,7 @@ nx_match_from_string_raw(const char *s, struct
> ofpbuf *b)
>  }
>  }
>  nw_header = htonll(header);
> -memcpy(header_ptr, &nw_header, nxm_header_len(header));
> +memcpy(b->header, &nw_header, nxm_header_len(header));
>
>  if (nxm_hasmask(header)) {
>  s += strspn(s, " ");
> --
> 2.1.4
>
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH] nx-match: Fix use-after-free parsing matches.

2016-03-07 Thread Joe Stringer
Address pointed by header_ptr might be free'd due to realloc
happened in ofpbuf_put_hex(). Reported by valgrind in the test
379: check TCP flags expression in OXM and NXM.

Invalid write of size 4
nx_match_from_string_raw (nx-match.c:1510)
nx_match_from_string (nx-match.c:1538)
ofctl_parse_nxm__ (ovs-ofctl.c:3325)
ovs_cmdl_run_command (command-line.c:121)
main (ovs-ofctl.c:137)

Address 0x7a2cc40 is 0 bytes inside a block of size 64 free'd
free (vg_replace_malloc.c:530)
ofpbuf_resize__ (ofpbuf.c:246)
ofpbuf_put (ofpbuf.c:386)
ofpbuf_put_hex (ofpbuf.c:414)
nx_match_from_string_raw (nx-match.c:1488)
nx_match_from_string (nx-match.c:1538)
ofctl_parse_nxm__ (ovs-ofctl.c:3325)

Reported-by: William Tu 
Signed-off-by: Joe Stringer 
---
 lib/nx-match.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/lib/nx-match.c b/lib/nx-match.c
index 4999b1ac95d7..6fcc53ad07d1 100644
--- a/lib/nx-match.c
+++ b/lib/nx-match.c
@@ -1467,7 +1467,6 @@ nx_match_from_string_raw(const char *s, struct ofpbuf *b)
 const char *name;
 uint64_t header;
 ovs_be64 nw_header;
-ovs_be64 *header_ptr;
 int name_len;
 size_t n;
 
@@ -1484,7 +1483,7 @@ nx_match_from_string_raw(const char *s, struct ofpbuf *b)
 
 s += name_len + 1;
 
-header_ptr = ofpbuf_put_uninit(b, nxm_header_len(header));
+b->header = ofpbuf_put_uninit(b, nxm_header_len(header));
 s = ofpbuf_put_hex(b, s, &n);
 if (n != nxm_field_bytes(header)) {
 const struct mf_field *field = mf_from_oxm_header(header);
@@ -1507,7 +1506,7 @@ nx_match_from_string_raw(const char *s, struct ofpbuf *b)
 }
 }
 nw_header = htonll(header);
-memcpy(header_ptr, &nw_header, nxm_header_len(header));
+memcpy(b->header, &nw_header, nxm_header_len(header));
 
 if (nxm_hasmask(header)) {
 s += strspn(s, " ");
-- 
2.1.4

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev