[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] slirp: Fix error reported by static code analys
From: |
Jan Kiszka |
Subject: |
Re: [Qemu-devel] [PATCH] slirp: Fix error reported by static code analysis and remove wrong type casts |
Date: |
Tue, 04 Sep 2012 09:27:35 +0200 |
User-agent: |
Mozilla/5.0 (X11; U; Linux i686 (x86_64); de; rv:1.8.1.12) Gecko/20080226 SUSE/2.0.0.12-1.1 Thunderbird/2.0.0.12 Mnenhy/0.7.5.666 |
On 2012-09-03 23:08, Peter Maydell wrote:
> On 3 September 2012 21:34, Stefan Weil <address@hidden> wrote:
>> Report from smatch:
>> slirp/tcp_subr.c:127 tcp_respond(17) error:
>> we previously assumed 'tp' could be null (see line 124)
>>
>> Fix this by checking 'tp' before reading its elements.
>>
>> The type casts of pointers to long are not related to the smatch report
>> but happened to be near that code. Those type casts are not allowed
>> when sizeof(pointer) != sizeof(long).
>
> I think these would be better in a separate patch.
>> @@ -124,7 +124,7 @@ tcp_respond(struct tcpcb *tp, struct tcpiphdr *ti,
>> struct mbuf *m,
>> if (tp)
>> win = sbspace(&tp->t_socket->so_rcv);
>> if (m == NULL) {
>> - if ((m = m_get(tp->t_socket->slirp)) == NULL)
>> + if (tp && (m = m_get(tp->t_socket->slirp)) == NULL)
>> return;
>> tlen = 0;
>> m->m_data += IF_MAXLINKHDR;
>
> This doesn't look quite right -- now if tp is NULL we will skip
> the assignment to m and dereference a NULL pointer a few lines
> further on, right?
This would be correct:
if (!tp || ..)
return
>
> I suspect that we need either to be passed our Slirp* explicitly rather
> than via tp, or we have to enforce that tcp_respond() is called with
> a non-NULL struct tcpcb*. I think the only cases where tp can be non-NULL
> at the moment are the two calls from the dropwithreset code in tcp_input().
Indeed, this is a "XXX Should never fail" case - according to the code
that checks tp at the call site. But as no one seriously understands
slirp details, we are better safe than sorry.
Jan
signature.asc
Description: OpenPGP digital signature