problem in if_tap.c

Hello,
I found the following problem in the if_tap-device code in
function tapcreate
when used on 64-bit systems:

       TAPDEBUG("tapcreate(%s%d). minor = %#xn",
name, unit, minor(dev));

        /* generate fake MAC address: 00 bd xx xx xx unit_no
*/
        macaddr_hi = htons(0x00bd);
        bcopy(&macaddr_hi, eaddr, sizeof(short));

---->
        bcopy(&ticks, &eaddr[2], sizeof(long));
        eaddr[5] = (u_char)unit;

        /* fill the rest and attach interface */

sizeof(long) is not always 4 on any system (e.g. on ia64
it's 8)
=> bytes are copied from undefined memory  into undefined
memory

Regards,
Marc

P.S.: On replies please cc me because I'm not on the list.
_______________________________________________
freebsd-netfreebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to
"freebsd-net-unsubscribefreebsd.org"

On Mon, Apr 14, 2008 at 1:33 AM, Marc Lörner
<marc.loernerhob.de> wrote:
> Hello,
>  I found the following problem in the if_tap-device
code in function tapcreate
>  when used on 64-bit systems:
>
>        TAPDEBUG("tapcreate(%s%d). minor =
%#xn", name, unit, minor(dev));
>
>         /* generate fake MAC address: 00 bd xx xx xx
unit_no */
>         macaddr_hi = htons(0x00bd);
>         bcopy(&macaddr_hi, eaddr, sizeof(short));
>
>  ---->
>         bcopy(&ticks, &eaddr[2],
sizeof(long));
>         eaddr[5] = (u_char)unit;
>
>         /* fill the rest and attach interface */
>
>  sizeof(long) is not always 4 on any system (e.g. on
ia64 it's 8)
>  => bytes are copied from undefined memory  into
undefined memory

please try the following patch. if there is no objections, i
will commit it

beetle# diff -u if_tap.c.orig if_tap.c
--- if_tap.c.orig       2007-04-05 10:58:39.000000000 -0700
+++ if_tap.c    2008-04-14 09:42:42.000000000 -0700
 -404,6
+404,7 
        struct ifnet            *ifp = NULL;
        struct tap_softc        *tp = NULL;
        unsigned short           macaddr_hi;
+       uint32_t                 macaddr_mid;
        int                      unit, s;
        char                    *name = NULL;
        u_char                  eaddr[6];
 -432,8
+433,9 

        /* generate fake MAC address: 00 bd xx xx xx unit_no
*/
        macaddr_hi = htons(0x00bd);
+       macaddr_mid = (uint32_t) ticks;
        bcopy(&macaddr_hi, eaddr, sizeof(short));
-       bcopy(&ticks, &eaddr[2], sizeof(long));
+       bcopy(&macaddr_mid, &eaddr[2],
sizeof(uint32_t));
        eaddr[5] = (u_char)unit;

        /* fill the rest and attach interface */

thanks,
max
_______________________________________________
freebsd-netfreebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to
"freebsd-net-unsubscribefreebsd.org"

Maksim Yevmenkin wrote:
> please try the following patch. if there is no
objections, i will commit it
>
> beetle# diff -u if_tap.c.orig if_tap.c
> --- if_tap.c.orig       2007-04-05 10:58:39.000000000
-0700
> +++ if_tap.c    2008-04-14 09:42:42.000000000 -0700
>  -404,6 +404,7 
>         struct ifnet            *ifp = NULL;
>         struct tap_softc        *tp = NULL;
>         unsigned short           macaddr_hi;
> +       uint32_t                 macaddr_mid;
>         int                      unit, s;
>         char                    *name = NULL;
>         u_char                  eaddr[6];
>  -432,8 +433,9 
>
>         /* generate fake MAC address: 00 bd xx xx xx
unit_no */
>         macaddr_hi = htons(0x00bd);
> +       macaddr_mid = (uint32_t) ticks;
>         bcopy(&macaddr_hi, eaddr, sizeof(short));
> -       bcopy(&ticks, &eaddr[2],
sizeof(long));
> +       bcopy(&macaddr_mid, &eaddr[2],
sizeof(uint32_t));
>         eaddr[5] = (u_char)unit;
>
>         /* fill the rest and attach interface */
>   


This patch looks good, please commit.


[Unless of course we want the autogenerated MAC to be
deterministic for 
some reason, but given that it comes from a timer, there's
not much 
point in fixing the endianness...]

cheers
BMS
_______________________________________________
freebsd-netfreebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to
"freebsd-net-unsubscribefreebsd.org"

On 4/14/08, Bruce M. Simpson <bmsfreebsd.org> wrote:
> Maksim Yevmenkin wrote:
>
> > please try the following patch. if there is no
objections, i will commit
> it

[...]

>
>  This patch looks good, please commit.

committed. thanks for the report and review.

thanks,
max
_______________________________________________
freebsd-netfreebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to
"freebsd-net-unsubscribefreebsd.org"