Hi,
@Matthias: As you correctly said &new->p.u.prefix points to a memory of 8
bytes, since union size is equal
to the size of maximum field and if someone writes to one field then it will
occupy this memory.
But struct in_addr is of 4 bytes long for an IPV4 address and we will
actually take decisions
inside prefix_bit from a position that actually is at offset 4, which will
probably will always
be "0". About IPV6 I do not understand why you say that the 1 extra byte we
will read will be still
in a valid memory area since then size of union u will be equal to 16 bytes
and we will read from
offset 16 when prefixlen is equal to 128 bits.
struct prefix
{
u_char family;
u_char prefixlen;
union
{
u_char prefix;
struct in_addr prefix4;
#ifdef HAVE_IPV6
struct in6_addr prefix6;
#endif /* HAVE_IPV6 */
struct
{
struct in_addr id;
struct in_addr adv_router;
} lp;
struct ethaddr prefix_eth; /* AF_ETHERNET */
u_char val[8];
uintptr_t ptr;
} u __attribute__ ((aligned (8)));
}
My point here is that in prefix_bit I would expect the offset to be equal
sizeof(struct in_addr) - 1
or sizeof (struct in6_addr) - 1 in order to be absolutely correct. It seems
that check_bit is a legacy
from zebra software. Please correct me if I miss anything here.
@ Dariusz: I am not so sure that we are so secure of not calling the
function with either prefixlen = 32 or 128.
Kostas Sotiropoulos
-----?????? ??????-----
???: quagga-dev-bounces@lists.quagga.net
[mailto:quagga-dev-bounces@lists.quagga.net] ?? ?????? ???
quagga-dev-request@lists.quagga.net
????????: Tuesday, July 14, 2020 2:00 PM
????: quagga-dev@lists.quagga.net
????: Quagga-dev Digest, Vol 188, Issue 2
Send Quagga-dev mailing list submissions to
quagga-dev@lists.quagga.net
To subscribe or unsubscribe via the World Wide Web, visit
https://lists.quagga.net/mailman/listinfo/quagga-dev You can reach the person managing the list at
quagga-dev-owner@lists.quagga.net
When replying, please edit your Subject line so it is more specific
than "Re: Contents of Quagga-dev digest..."
Today's Topics:
1. [quagga-dev 16763] Re: possible problem with prefix_bit
(Matthias Ferdinand)
2. [quagga-dev 16764] ODP: Re: possible problem with prefix_bit
(Dariusz Grzeg?rski)
----------------------------------------------------------------------
Message: 1
Date: Mon, 13 Jul 2020 14:29:50 +0200
From: Matthias Ferdinand <mf@14v.de>
To: quagga-dev@lists.quagga.net
Subject: [quagga-dev 16763] Re: possible problem with prefix_bit
Message-ID: <20200713122950.GE23382@xoff>
Content-Type: text/plain; charset=utf-8
On Mon, Jul 13, 2020 at 12:00:02PM +0100,
quagga-dev-request@lists.quagga.net wrote:
> Message: 1
> Date: Sun, 12 Jul 2020 21:22:15 +0000 (UTC)
> From: Kostas Sotiropoulos <kosotiro@yahoo.gr>
> To: "quagga-dev@lists.quagga.net" <quagga-dev@lists.quagga.net>
> Subject: [quagga-dev 16762] possible problem with prefix_bit
> Message-ID: <1148511041.807225.1594588935714@mail.yahoo.com>
> Content-Type: text/plain; charset="utf-8"
>
> Hi all,
>
> I do not know if this list is still valid but anyway I will express my
anxiety for a code snippet:
> Inside lib/table.c there is function set_link:
> static void
> set_link (struct route_node *node, struct route_node *new)
> {
> ? unsigned int bit = prefix_bit (&new->p.u.prefix, node->p.prefixlen);
>
> ? node->link[bit] = new;
> ? new->parent = node;
> }
> that calls function prefix_bit:
> unsigned int
> prefix_bit (const u_char *prefix, const u_char prefixlen)
> {
> ? unsigned int offset = prefixlen / 8;
> ? unsigned int shift? = 7 - (prefixlen % 8);
> ?
> ? return (prefix[offset] >> shift) & 1;
> }
>
> I suppose that prefixlen could also be equal to 32 for an IPV4 address
that could result to a buffer overrun insideprefix_bit. Am I right?
> Best regards,Kostas Sotiropoulos
[ Disclaimer: I'm not a developer ]
Hi,
note that struct prefix is at least 8 bytes long, and AFAICT is usually
embedded within a larger struct (e.g. struct route_node) with more
components following after struct prefix.
With 8 bytes size, prefixlen==32 for an IPv4 address will not read from
outside struct prefix (offset==4).
With prefixlen==128 for an IPv6 address it might read 1 byte after struct
prefix (offset==8), but still from valid memory.
Matthias Ferdinand
------------------------------
Message: 2
Date: Mon, 13 Jul 2020 13:34:17 +0000
From: Dariusz Grzeg?rski <dariusz.grzegorski@transbit.com.pl>
To: "quagga-dev@lists.quagga.net" <quagga-dev@lists.quagga.net>
Subject: [quagga-dev 16764] ODP: Re: possible problem with prefix_bit
Message-ID:
<AM0PR04MB43371E652506CC4518FBDCF6CF600@AM0PR04MB4337.eurprd04.prod.outlook.
Content-Type: text/plain; charset="iso-8859-2"
I suppose the prerequisite for calling set_link() is that new node's prefix
should be longer than its parent's one (but still valid) - it's a rather
obvious way of building a prefix tree. So, prefix_bit() will always read
valid data.
________________________________
Od: quagga-dev-bounces@lists.quagga.net
<quagga-dev-bounces@lists.quagga.net> w imieniu u?ytkownika Matthias
Ferdinand <mf@14v.de>
Wys?ane: poniedzia?ek, 13 lipca 2020 14:29
Do: quagga-dev@lists.quagga.net <quagga-dev@lists.quagga.net>
Temat: [quagga-dev 16763] Re: possible problem with prefix_bit
On Mon, Jul 13, 2020 at 12:00:02PM +0100,
quagga-dev-request@lists.quagga.net wrote:
> Message: 1
> Date: Sun, 12 Jul 2020 21:22:15 +0000 (UTC)
> From: Kostas Sotiropoulos <kosotiro@yahoo.gr>
> To: "quagga-dev@lists.quagga.net" <quagga-dev@lists.quagga.net>
> Subject: [quagga-dev 16762] possible problem with prefix_bit
> Message-ID: <1148511041.807225.1594588935714@mail.yahoo.com>
> Content-Type: text/plain; charset="utf-8"
>
> Hi all,
>
> I do not know if this list is still valid but anyway I will express my
anxiety for a code snippet:
> Inside lib/table.c there is function set_link:
> static void
> set_link (struct route_node *node, struct route_node *new)
> {
> ? unsigned int bit = prefix_bit (&new->p.u.prefix, node->p.prefixlen);
>
> ? node->link[bit] = new;
> ? new->parent = node;
> }
> that calls function prefix_bit:
> unsigned int
> prefix_bit (const u_char *prefix, const u_char prefixlen)
> {
> ? unsigned int offset = prefixlen / 8;
> ? unsigned int shift? = 7 - (prefixlen % 8);
> ?
> ? return (prefix[offset] >> shift) & 1;
> }
>
> I suppose that prefixlen could also be equal to 32 for an IPV4 address
that could result to a buffer overrun insideprefix_bit. Am I right?
> Best regards,Kostas Sotiropoulos
[ Disclaimer: I'm not a developer ]
Hi,
note that struct prefix is at least 8 bytes long, and AFAICT is usually
embedded within a larger struct (e.g. struct route_node) with more
components following after struct prefix.
With 8 bytes size, prefixlen==32 for an IPv4 address will not read from
outside struct prefix (offset==4).
With prefixlen==128 for an IPv6 address it might read 1 byte after struct
prefix (offset==8), but still from valid memory.
Matthias Ferdinand
_______________________________________________
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev -------------- next part --------------
An HTML attachment was scrubbed...
URL:
<
http://lists.quagga.net/pipermail/quagga-dev/attachments/20200713/63da33f8/ attachment-0001.html>
------------------------------
_______________________________________________
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev End of Quagga-dev Digest, Vol 188, Issue 2
******************************************
_______________________________________________
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev