| Message ID | 20140611085553.GO31888@type |
|---|---|
| State | New |
| Headers | show |
On 2014-06-11 10:55, Samuel Thibault wrote: > Before this patch, if sosendto fails, udp_input is executed as if the > packet was sent. This could cause memory leak. Cannot follow yet how this could leak (not saying I fully got what it should NOT leak - nasty code). Can you elaborate on the before/after? Jan > This patch adds a goto bad to cut the execution of this function. > > Signed-off-by: Guillaume Subiron <maethor@subiron.org> > --- > slirp/udp.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/slirp/udp.c b/slirp/udp.c > index 8cc6cb6..fd2446a 100644 > --- a/slirp/udp.c > +++ b/slirp/udp.c > @@ -218,6 +218,7 @@ udp_input(register struct mbuf *m, int iphlen) > *ip=save_ip; > DEBUG_MISC((dfd,"udp tx errno = %d-%s\n",errno,strerror(errno))); > icmp_error(m, ICMP_UNREACH,ICMP_UNREACH_NET, 0,strerror(errno)); > + goto bad; > } > > m_free(so->so_m); /* used for ICMP if error on sorecvfrom */ >
Jan Kiszka, le Thu 12 Jun 2014 07:47:25 +0200, a écrit : > On 2014-06-11 10:55, Samuel Thibault wrote: > > Before this patch, if sosendto fails, udp_input is executed as if the > > packet was sent. This could cause memory leak. > > Cannot follow yet how this could leak (not saying I fully got what it > should NOT leak - nasty code). Can you elaborate on the before/after? I haven't worked on the patch, but can comment a bit. I'm not sure it's actually a memory leak, but the "before" situation is quite confusing actually :) Before, m->m_len += iphlen and m->m_data -= iphlen would be done twice in the end, thus leaving m in an odd state. At any rate, letting udp_input put m into so->so_m does not make any sense: so->so_m is used by icmp_receive/sorecvfrom to know where to send back any errors from the outside for a packet that we have emitted. Here, since we haven't actually emitted the packet, there is not much sense in using it, and any error that we may get later would rather be related to the previous packet, not the one we haven't emitted. Samuel
diff --git a/slirp/udp.c b/slirp/udp.c index 8cc6cb6..fd2446a 100644 --- a/slirp/udp.c +++ b/slirp/udp.c @@ -218,6 +218,7 @@ udp_input(register struct mbuf *m, int iphlen) *ip=save_ip; DEBUG_MISC((dfd,"udp tx errno = %d-%s\n",errno,strerror(errno))); icmp_error(m, ICMP_UNREACH,ICMP_UNREACH_NET, 0,strerror(errno)); + goto bad; } m_free(so->so_m); /* used for ICMP if error on sorecvfrom */
Before this patch, if sosendto fails, udp_input is executed as if the packet was sent. This could cause memory leak. This patch adds a goto bad to cut the execution of this function. Signed-off-by: Guillaume Subiron <maethor@subiron.org> --- slirp/udp.c | 1 + 1 file changed, 1 insertion(+)