| Message ID | 1682070478-23812-1-git-send-email-wangyunjian@huawei.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev,v3] dpif-netlink: Fix memory leak dpif_netlink_open(). | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/intel-ovs-compilation | success | test: success |
On Fri, Apr 21, 2023 at 05:47:58PM +0800, Yunjian Wang wrote: > In the specific call to dpif_netlink_dp_transact() (line 398) in > dpif_netlink_open(), the 'dp' content is not being used in the branch > when no error is returned (starting line 430). Furthermore, the 'dp' > and 'buf' variables are overwritten later in this same branch when a > new netlink request is sent (line 437), which results in a memory leak. > > Reported by Address Sanitizer. > > Indirect leak of 1024 byte(s) in 1 object(s) allocated from: > #0 0x7fe09d3bfe70 in __interceptor_malloc (/usr/lib64/libasan.so.4+0xe0e70) > #1 0x8759be in xmalloc__ lib/util.c:140 > #2 0x875a9a in xmalloc lib/util.c:175 > #3 0x7ba0d2 in ofpbuf_init lib/ofpbuf.c:141 > #4 0x7ba1d6 in ofpbuf_new lib/ofpbuf.c:169 > #5 0x9057f9 in nl_sock_transact lib/netlink-socket.c:1113 > #6 0x907a7e in nl_transact lib/netlink-socket.c:1817 > #7 0x8b5abe in dpif_netlink_dp_transact lib/dpif-netlink.c:5007 > #8 0x89a6b5 in dpif_netlink_open lib/dpif-netlink.c:398 > #9 0x5de16f in do_open lib/dpif.c:348 > #10 0x5de69a in dpif_open lib/dpif.c:393 > #11 0x5de71f in dpif_create_and_open lib/dpif.c:419 > #12 0x47b918 in open_dpif_backer ofproto/ofproto-dpif.c:764 > #13 0x483e4a in construct ofproto/ofproto-dpif.c:1658 > #14 0x441644 in ofproto_create ofproto/ofproto.c:556 > #15 0x40ba5a in bridge_reconfigure vswitchd/bridge.c:885 > #16 0x41f1a9 in bridge_run vswitchd/bridge.c:3313 > #17 0x42d4fb in main vswitchd/ovs-vswitchd.c:132 > #18 0x7fe09cc03c86 in __libc_start_main (/usr/lib64/libc.so.6+0x25c86) > > Fixes: b841e3cd4a28 ("dpif-netlink: Fix feature negotiation for older kernels.") > Suggested-by: David Marchand <david.marchand@redhat.com> > Signed-off-by: Yunjian Wang <wangyunjian@huawei.com> > --- > v3: update commit log Thanks for the update. Reviewed-by: Simon Horman <simon.horman@corigine.com>
Hello, On Fri, Apr 21, 2023 at 11:48 AM Yunjian Wang <wangyunjian@huawei.com> wrote: > > In the specific call to dpif_netlink_dp_transact() (line 398) in > dpif_netlink_open(), the 'dp' content is not being used in the branch > when no error is returned (starting line 430). Furthermore, the 'dp' > and 'buf' variables are overwritten later in this same branch when a > new netlink request is sent (line 437), which results in a memory leak. > > Reported by Address Sanitizer. > > Indirect leak of 1024 byte(s) in 1 object(s) allocated from: > #0 0x7fe09d3bfe70 in __interceptor_malloc (/usr/lib64/libasan.so.4+0xe0e70) > #1 0x8759be in xmalloc__ lib/util.c:140 > #2 0x875a9a in xmalloc lib/util.c:175 > #3 0x7ba0d2 in ofpbuf_init lib/ofpbuf.c:141 > #4 0x7ba1d6 in ofpbuf_new lib/ofpbuf.c:169 > #5 0x9057f9 in nl_sock_transact lib/netlink-socket.c:1113 > #6 0x907a7e in nl_transact lib/netlink-socket.c:1817 > #7 0x8b5abe in dpif_netlink_dp_transact lib/dpif-netlink.c:5007 > #8 0x89a6b5 in dpif_netlink_open lib/dpif-netlink.c:398 > #9 0x5de16f in do_open lib/dpif.c:348 > #10 0x5de69a in dpif_open lib/dpif.c:393 > #11 0x5de71f in dpif_create_and_open lib/dpif.c:419 > #12 0x47b918 in open_dpif_backer ofproto/ofproto-dpif.c:764 > #13 0x483e4a in construct ofproto/ofproto-dpif.c:1658 > #14 0x441644 in ofproto_create ofproto/ofproto.c:556 > #15 0x40ba5a in bridge_reconfigure vswitchd/bridge.c:885 > #16 0x41f1a9 in bridge_run vswitchd/bridge.c:3313 > #17 0x42d4fb in main vswitchd/ovs-vswitchd.c:132 > #18 0x7fe09cc03c86 in __libc_start_main (/usr/lib64/libc.so.6+0x25c86) > > Fixes: b841e3cd4a28 ("dpif-netlink: Fix feature negotiation for older kernels.") > Suggested-by: David Marchand <david.marchand@redhat.com> I only suggested a change in the patch as I was reviewing it. I'd prefer this is replaced with Reviewed-by: (and I confirm the patch lgtm). > Signed-off-by: Yunjian Wang <wangyunjian@huawei.com> Thanks.
> -----Original Message----- > From: David Marchand [mailto:david.marchand@redhat.com] > Sent: Monday, April 24, 2023 4:20 PM > To: wangyunjian <wangyunjian@huawei.com> > Cc: dev@openvswitch.org; i.maximets@ovn.org; luyicai > <luyicai@huawei.com>; simon.horman@corigine.com > Subject: Re: [ovs-dev] [PATCH v3] dpif-netlink: Fix memory leak > dpif_netlink_open(). > > Hello, > > On Fri, Apr 21, 2023 at 11:48 AM Yunjian Wang <wangyunjian@huawei.com> > wrote: > > > > In the specific call to dpif_netlink_dp_transact() (line 398) in > > dpif_netlink_open(), the 'dp' content is not being used in the branch > > when no error is returned (starting line 430). Furthermore, the 'dp' > > and 'buf' variables are overwritten later in this same branch when a > > new netlink request is sent (line 437), which results in a memory leak. > > > > Reported by Address Sanitizer. > > > > Indirect leak of 1024 byte(s) in 1 object(s) allocated from: > > #0 0x7fe09d3bfe70 in __interceptor_malloc > (/usr/lib64/libasan.so.4+0xe0e70) > > #1 0x8759be in xmalloc__ lib/util.c:140 > > #2 0x875a9a in xmalloc lib/util.c:175 > > #3 0x7ba0d2 in ofpbuf_init lib/ofpbuf.c:141 > > #4 0x7ba1d6 in ofpbuf_new lib/ofpbuf.c:169 > > #5 0x9057f9 in nl_sock_transact lib/netlink-socket.c:1113 > > #6 0x907a7e in nl_transact lib/netlink-socket.c:1817 > > #7 0x8b5abe in dpif_netlink_dp_transact lib/dpif-netlink.c:5007 > > #8 0x89a6b5 in dpif_netlink_open lib/dpif-netlink.c:398 > > #9 0x5de16f in do_open lib/dpif.c:348 > > #10 0x5de69a in dpif_open lib/dpif.c:393 > > #11 0x5de71f in dpif_create_and_open lib/dpif.c:419 > > #12 0x47b918 in open_dpif_backer ofproto/ofproto-dpif.c:764 > > #13 0x483e4a in construct ofproto/ofproto-dpif.c:1658 > > #14 0x441644 in ofproto_create ofproto/ofproto.c:556 > > #15 0x40ba5a in bridge_reconfigure vswitchd/bridge.c:885 > > #16 0x41f1a9 in bridge_run vswitchd/bridge.c:3313 > > #17 0x42d4fb in main vswitchd/ovs-vswitchd.c:132 > > #18 0x7fe09cc03c86 in __libc_start_main > > (/usr/lib64/libc.so.6+0x25c86) > > > > Fixes: b841e3cd4a28 ("dpif-netlink: Fix feature negotiation for older > > kernels.") > > Suggested-by: David Marchand <david.marchand@redhat.com> > > I only suggested a change in the patch as I was reviewing it. > I'd prefer this is replaced with Reviewed-by: (and I confirm the patch lgtm). OK, I will update it. Thanks, Yunjian > > > Signed-off-by: Yunjian Wang <wangyunjian@huawei.com> > > Thanks. > > > -- > David Marchand
diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c index de0711277..60bd39643 100644 --- a/lib/dpif-netlink.c +++ b/lib/dpif-netlink.c @@ -395,7 +395,7 @@ dpif_netlink_open(const struct dpif_class *class OVS_UNUSED, const char *name, dp_request.user_features |= OVS_DP_F_UNALIGNED; dp_request.user_features |= OVS_DP_F_VPORT_PIDS; dp_request.user_features |= OVS_DP_F_UNSUPPORTED; - error = dpif_netlink_dp_transact(&dp_request, &dp, &buf); + error = dpif_netlink_dp_transact(&dp_request, NULL, NULL); if (error) { /* The Open vSwitch kernel module has two modes for dispatching * upcalls: per-vport and per-cpu.
In the specific call to dpif_netlink_dp_transact() (line 398) in dpif_netlink_open(), the 'dp' content is not being used in the branch when no error is returned (starting line 430). Furthermore, the 'dp' and 'buf' variables are overwritten later in this same branch when a new netlink request is sent (line 437), which results in a memory leak. Reported by Address Sanitizer. Indirect leak of 1024 byte(s) in 1 object(s) allocated from: #0 0x7fe09d3bfe70 in __interceptor_malloc (/usr/lib64/libasan.so.4+0xe0e70) #1 0x8759be in xmalloc__ lib/util.c:140 #2 0x875a9a in xmalloc lib/util.c:175 #3 0x7ba0d2 in ofpbuf_init lib/ofpbuf.c:141 #4 0x7ba1d6 in ofpbuf_new lib/ofpbuf.c:169 #5 0x9057f9 in nl_sock_transact lib/netlink-socket.c:1113 #6 0x907a7e in nl_transact lib/netlink-socket.c:1817 #7 0x8b5abe in dpif_netlink_dp_transact lib/dpif-netlink.c:5007 #8 0x89a6b5 in dpif_netlink_open lib/dpif-netlink.c:398 #9 0x5de16f in do_open lib/dpif.c:348 #10 0x5de69a in dpif_open lib/dpif.c:393 #11 0x5de71f in dpif_create_and_open lib/dpif.c:419 #12 0x47b918 in open_dpif_backer ofproto/ofproto-dpif.c:764 #13 0x483e4a in construct ofproto/ofproto-dpif.c:1658 #14 0x441644 in ofproto_create ofproto/ofproto.c:556 #15 0x40ba5a in bridge_reconfigure vswitchd/bridge.c:885 #16 0x41f1a9 in bridge_run vswitchd/bridge.c:3313 #17 0x42d4fb in main vswitchd/ovs-vswitchd.c:132 #18 0x7fe09cc03c86 in __libc_start_main (/usr/lib64/libc.so.6+0x25c86) Fixes: b841e3cd4a28 ("dpif-netlink: Fix feature negotiation for older kernels.") Suggested-by: David Marchand <david.marchand@redhat.com> Signed-off-by: Yunjian Wang <wangyunjian@huawei.com> --- v3: update commit log --- lib/dpif-netlink.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)