* Problem report #15
@ 2006-04-11 15:48 Dan Nicolaescu
2006-04-11 17:44 ` Stuart D. Herring
0 siblings, 1 reply; 6+ messages in thread
From: Dan Nicolaescu @ 2006-04-11 15:48 UTC (permalink / raw
CID: 15
Checker: FORWARD_NULL (help)
File: emacs/src/process.c
Function: conv_lisp_to_sockaddr
Description: Incrementing possibly NULL value "cp"
Event assign_zero: Variable "cp" assigned value 0.
Also see events: [dereference]
2335 register unsigned char *cp = NULL;
2336 register int i;
2337
2338 bzero (sa, len);
2339 sa->sa_family = family;
2340
At conditional (1): "address & 7 == 4" taking true path
At conditional (2): "((0), (address & -8))->size & 1073741824 == 0" taking true path
2341 if (VECTORP (address))
2342 {
2343 p = XVECTOR (address);
At conditional (3): "family == 2" taking false path
2344 if (family == AF_INET)
2345 {
2346 struct sockaddr_in *sin = (struct sockaddr_in *) sa;
2347 len = sizeof (sin->sin_addr) + 1;
2348 i = XINT (p->contents[--len]);
2349 sin->sin_port = htons (i);
2350 cp = (unsigned char *)&sin->sin_addr;
2351 }
2352 #ifdef AF_INET6
At conditional (4): "family == 10" taking false path
2353 else if (family == AF_INET6)
2354 {
2355 struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *) sa;
2356 uint16_t *ip6 = (uint16_t *)&sin6->sin6_addr;
2357 len = sizeof (sin6->sin6_addr) + 1;
2358 i = XINT (p->contents[--len]);
2359 sin6->sin6_port = htons (i);
2360 for (i = 0; i < len; i++)
2361 if (INTEGERP (p->contents[i]))
2362 {
2363 int j = XFASTINT (p->contents[i]) & 0xffff;
2364 ip6[i] = ntohs (j);
2365 }
2366 return;
2367 }
2368 #endif
2369 }
2370 else if (STRINGP (address))
2371 {
2372 #ifdef HAVE_LOCAL_SOCKETS
2373 if (family == AF_LOCAL)
2374 {
2375 struct sockaddr_un *sockun = (struct sockaddr_un *) sa;
2376 cp = SDATA (address);
2377 for (i = 0; i < sizeof (sockun->sun_path) && *cp; i++)
2378 sockun->sun_path[i] = *cp++;
2379 }
2380 #endif
2381 return;
2382 }
2383 else
2384 {
2385 p = XVECTOR (XCDR (address));
2386 cp = (unsigned char *)sa + sizeof (sa->sa_family);
2387 }
2388
At conditional (5): "i < len" taking true path
2389 for (i = 0; i < len; i++)
At conditional (6): "(p)->contents[i] & 7 == 0" taking true path
2390 if (INTEGERP (p->contents[i]))
Event dereference: Incrementing possibly NULL value "cp"
Also see events: [assign_zero]
2391 *cp++ = XFASTINT (p->contents[i]) & 0xff;
2392 }
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Problem report #15
2006-04-11 15:48 Problem report #15 Dan Nicolaescu
@ 2006-04-11 17:44 ` Stuart D. Herring
2006-04-12 17:10 ` Richard Stallman
0 siblings, 1 reply; 6+ messages in thread
From: Stuart D. Herring @ 2006-04-11 17:44 UTC (permalink / raw
> 2344 if (family == AF_INET)
> 2345 {
> [...]
> 2351 }
> 2352 #ifdef AF_INET6
>
> At conditional (4): "family == 10" taking false path
>
> 2353 else if (family == AF_INET6)
> 2354 {
> [...]
> 2367 }
> 2368 #endif
> 2369 }
Shouldn't there be some sort of else signal bad-family-choice? It looks
like get_lisp_to_sockaddr_size can store anything into family...
> At conditional (5): "i < len" taking true path
>
> 2389 for (i = 0; i < len; i++)
>
> At conditional (6): "(p)->contents[i] & 7 == 0" taking true path
>
> 2390 if (INTEGERP (p->contents[i]))
>
> Event dereference: Incrementing possibly NULL value "cp"
> Also see events: [assign_zero]
>
> 2391 *cp++ = XFASTINT (p->contents[i]) & 0xff;
For that matter, shouldn't there be some sort of signal when there are
things that aren't integers present? I really don't expect
[save-buffers-kill-emacs 64 #<subr symbol-function> 241]
to silently become 64.241.?.?. I think this is, perhaps, two bugs.
Davis
--
This product is sold by volume, not by mass. If it appears too dense or
too sparse, it is because mass-energy conversion has occurred during
shipping.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Problem report #15
2006-04-11 17:44 ` Stuart D. Herring
@ 2006-04-12 17:10 ` Richard Stallman
2006-04-12 17:36 ` Stuart D. Herring
0 siblings, 1 reply; 6+ messages in thread
From: Richard Stallman @ 2006-04-12 17:10 UTC (permalink / raw
Cc: emacs-devel
FAMILY can't be invalid because get_lisp_to_sockaddr_size
has made sure it is valid.
(The comment said size_lisp_to_sockaddr; I fixed that.)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Problem report #15
2006-04-12 17:10 ` Richard Stallman
@ 2006-04-12 17:36 ` Stuart D. Herring
2006-04-13 3:20 ` Richard Stallman
0 siblings, 1 reply; 6+ messages in thread
From: Stuart D. Herring @ 2006-04-12 17:36 UTC (permalink / raw
Cc: emacs-devel
> FAMILY can't be invalid because get_lisp_to_sockaddr_size
> has made sure it is valid.
>
> (The comment said size_lisp_to_sockaddr; I fixed that.)
I don't see how get_lisp_to_sockaddr_size does that. If `address' is a
Lisp vector but has length, say, 15, it seems that `family' (`familyp') is
never assigned. As far as I can tell, no one ever checks the value of
family between there and where it is compared to AF_INET and AF_INET6. If
it compares unequal to each of those, then `cp' in conv_lisp_to_sockaddr
will be left NULL and the last line in conv_lisp_to_sockaddr will
segfault.
I think this bug is real, then -- probably just need to signal a Lisp
error in conv_lisp_to_sockaddr if the address is a vector but family is
neither AF_INET nor AF_INET6.
Davis
--
This product is sold by volume, not by mass. If it appears too dense or
too sparse, it is because mass-energy conversion has occurred during
shipping.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Problem report #15
2006-04-12 17:36 ` Stuart D. Herring
@ 2006-04-13 3:20 ` Richard Stallman
2006-04-13 15:28 ` Stuart D. Herring
0 siblings, 1 reply; 6+ messages in thread
From: Richard Stallman @ 2006-04-13 3:20 UTC (permalink / raw
Cc: emacs-devel
I don't see how get_lisp_to_sockaddr_size does that. If `address' is a
Lisp vector but has length, say, 15, it seems that `family' (`familyp') is
never assigned. As far as I can tell, no one ever checks the value of
family between there and where it is compared to AF_INET and AF_INET6.
Yes, you're right. I think I have fixed it now.
However, I wonder, why not make conv_lisp_to_sockaddr signal
a Lisp error if the family is not recognized. Is there any
reason not to do this?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Problem report #15
2006-04-13 3:20 ` Richard Stallman
@ 2006-04-13 15:28 ` Stuart D. Herring
0 siblings, 0 replies; 6+ messages in thread
From: Stuart D. Herring @ 2006-04-13 15:28 UTC (permalink / raw
Cc: emacs-devel
> Yes, you're right. I think I have fixed it now.
>
> However, I wonder, why not make conv_lisp_to_sockaddr signal
> a Lisp error if the family is not recognized. Is there any
> reason not to do this?
That's exactly what I suggested in the previous mail. However, it might
be good for get_lisp_to_sockaddr_size to signal if it can't figure out a
reasonable size, so that code doesn't break in the future if a new address
family or so is added to one and not the other.
Davis
--
This product is sold by volume, not by mass. If it appears too dense or
too sparse, it is because mass-energy conversion has occurred during
shipping.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2006-04-13 15:28 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-04-11 15:48 Problem report #15 Dan Nicolaescu
2006-04-11 17:44 ` Stuart D. Herring
2006-04-12 17:10 ` Richard Stallman
2006-04-12 17:36 ` Stuart D. Herring
2006-04-13 3:20 ` Richard Stallman
2006-04-13 15:28 ` Stuart D. Herring
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).