Re: [ovs-dev] [PATCH] table: Fix freeing global variable.

2024-05-14 Thread wangyunjian via dev 



> -Original Message-
> From: Ilya Maximets [mailto:i.maxim...@ovn.org]
> Sent: Tuesday, May 14, 2024 3:48 PM
> To: wangyunjian ; d...@openvswitch.org
> Cc: i.maxim...@ovn.org; luyicai ; sunpengfei (G)
> 
> Subject: Re: [ovs-dev][PATCH] table: Fix freeing global variable.
> 
> On 5/14/24 05:25, Yunjian Wang wrote:
> > From: Pengfei Sun 
> >
> > In function shash_replace_nocopy, argument to free() is the address of
> > a global variable (argument passed by function table_print_json__),
> > which is not memory allocated by malloc().
> >
> > ovsdb-client -f json monitor Open_vSwitch --timestamp
> >
> > ASan reports:
> >
> 
> =
> > ==1443083==ERROR: AddressSanitizer: attempting free on address which
> > was not malloc()-ed: 0x00535980 in thread T0
> > #0 0xaf1c9eac in __interceptor_free (/usr/lib64/libasan.so.6+
> > 0xa4eac)
> > #1 0x4826e4 in json_destroy_object lib/json.c:445
> > #2 0x4826e4 in json_destroy__ lib/json.c:403
> > #3 0x4cc4e4 in table_print lib/table.c:633
> > #4 0x410650 in monitor_print_table ovsdb/ovsdb-client.c:1019
> > #5 0x410650 in monitor_print ovsdb/ovsdb-client.c:1040
> > #6 0x4110cc in monitor_print ovsdb/ovsdb-client.c:1030
> > #7 0x4110cc in do_monitor__ ovsdb/ovsdb-client.c:1503
> > #8 0x40743c in main ovsdb/ovsdb-client.c:283
> > #9 0xaeb50038  (/usr/lib64/libc.so.6+0x2b038)
> > #10 0xaeb50110 in __libc_start_main (/usr/lib64/libc.so.6+
> > 0x2b110)
> > #11 0x40906c in _start (/usr/local/bin/ovsdb-client+0x40906c)
> >
> > Use xstrdup to allocate memory for argument "time".
> >
> > Fixes: cb139fa8b3a1 ("table: New function table_format() for
> > formatting a table as a string.")
> > Signed-off-by: Pengfei Sun 
> > ---
> >  lib/table.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Hi.  Thanks for the patch!
> 
> >
> > diff --git a/lib/table.c b/lib/table.c index 48d18b6..bcc6fb0 100644
> > --- a/lib/table.c
> > +++ b/lib/table.c
> > @@ -523,7 +523,7 @@ table_print_json__(const struct table *table, const
> struct table_style *style,
> >  }
> >  if (table->timestamp) {
> >  json_object_put_nocopy(
> 
> I think, we should just remove 'nocopy' part here instead of adding xstrdup()
> below.  json_object_put() will copy the name.

Thanks for your suggestion, will include it in next version.

> 
> > -json, "time",
> > +json, xstrdup("time"),
> >  json_string_create_nocopy(table_format_timestamp__()));
> >  }
> >
> 
> Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] table: Fix freeing global variable.

2024-05-14 Thread Ilya Maximets 
On 5/14/24 05:25, Yunjian Wang wrote:
> From: Pengfei Sun 
> 
> In function shash_replace_nocopy, argument to free() is the address
> of a global variable (argument passed by function table_print_json__),
> which is not memory allocated by malloc().
> 
> ovsdb-client -f json monitor Open_vSwitch --timestamp
> 
> ASan reports:
> =
> ==1443083==ERROR: AddressSanitizer: attempting free on address
> which was not malloc()-ed: 0x00535980 in thread T0
> #0 0xaf1c9eac in __interceptor_free (/usr/lib64/libasan.so.6+
> 0xa4eac)
> #1 0x4826e4 in json_destroy_object lib/json.c:445
> #2 0x4826e4 in json_destroy__ lib/json.c:403
> #3 0x4cc4e4 in table_print lib/table.c:633
> #4 0x410650 in monitor_print_table ovsdb/ovsdb-client.c:1019
> #5 0x410650 in monitor_print ovsdb/ovsdb-client.c:1040
> #6 0x4110cc in monitor_print ovsdb/ovsdb-client.c:1030
> #7 0x4110cc in do_monitor__ ovsdb/ovsdb-client.c:1503
> #8 0x40743c in main ovsdb/ovsdb-client.c:283
> #9 0xaeb50038  (/usr/lib64/libc.so.6+0x2b038)
> #10 0xaeb50110 in __libc_start_main (/usr/lib64/libc.so.6+
> 0x2b110)
> #11 0x40906c in _start (/usr/local/bin/ovsdb-client+0x40906c)
> 
> Use xstrdup to allocate memory for argument "time".
> 
> Fixes: cb139fa8b3a1 ("table: New function table_format() for formatting a 
> table as a string.")
> Signed-off-by: Pengfei Sun 
> ---
>  lib/table.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Hi.  Thanks for the patch!

> 
> diff --git a/lib/table.c b/lib/table.c
> index 48d18b6..bcc6fb0 100644
> --- a/lib/table.c
> +++ b/lib/table.c
> @@ -523,7 +523,7 @@ table_print_json__(const struct table *table, const 
> struct table_style *style,
>  }
>  if (table->timestamp) {
>  json_object_put_nocopy(

I think, we should just remove 'nocopy' part here instead of
adding xstrdup() below.  json_object_put() will copy the name.

> -json, "time",
> +json, xstrdup("time"),
>  json_string_create_nocopy(table_format_timestamp__()));
>  }
>  

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] table: Fix freeing global variable.

2024-05-13 Thread Yunjian Wang via dev 
From: Pengfei Sun 

In function shash_replace_nocopy, argument to free() is the address
of a global variable (argument passed by function table_print_json__),
which is not memory allocated by malloc().

ovsdb-client -f json monitor Open_vSwitch --timestamp

ASan reports:
=
==1443083==ERROR: AddressSanitizer: attempting free on address
which was not malloc()-ed: 0x00535980 in thread T0
#0 0xaf1c9eac in __interceptor_free (/usr/lib64/libasan.so.6+
0xa4eac)
#1 0x4826e4 in json_destroy_object lib/json.c:445
#2 0x4826e4 in json_destroy__ lib/json.c:403
#3 0x4cc4e4 in table_print lib/table.c:633
#4 0x410650 in monitor_print_table ovsdb/ovsdb-client.c:1019
#5 0x410650 in monitor_print ovsdb/ovsdb-client.c:1040
#6 0x4110cc in monitor_print ovsdb/ovsdb-client.c:1030
#7 0x4110cc in do_monitor__ ovsdb/ovsdb-client.c:1503
#8 0x40743c in main ovsdb/ovsdb-client.c:283
#9 0xaeb50038  (/usr/lib64/libc.so.6+0x2b038)
#10 0xaeb50110 in __libc_start_main (/usr/lib64/libc.so.6+
0x2b110)
#11 0x40906c in _start (/usr/local/bin/ovsdb-client+0x40906c)

Use xstrdup to allocate memory for argument "time".

Fixes: cb139fa8b3a1 ("table: New function table_format() for formatting a table 
as a string.")
Signed-off-by: Pengfei Sun 
---
 lib/table.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/table.c b/lib/table.c
index 48d18b6..bcc6fb0 100644
--- a/lib/table.c
+++ b/lib/table.c
@@ -523,7 +523,7 @@ table_print_json__(const struct table *table, const struct 
table_style *style,
 }
 if (table->timestamp) {
 json_object_put_nocopy(
-json, "time",
+json, xstrdup("time"),
 json_string_create_nocopy(table_format_timestamp__()));
 }
 
-- 
2.33.0

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev