[src/trunk]: src/sys/netatalk atalk(4): Don't let userland control sa_len whe...

[riastradh <riastradh%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/9c2609380d31
branches:  trunk
changeset: 374082:9c2609380d31
user:      riastradh <riastradh%NetBSD.org@localhost>
date:      Thu Mar 30 15:58:10 2023 +0000

description:
atalk(4): Don't let userland control sa_len when adding addresses.

- The struct sockaddr_at object accessed by at_ifinit always comes
  from an ioctl buffer that is adequately sized and fully initialized
  from userland memory.

- The newly created sockaddr_at object is later used with
  sockaddr_copy, which requires sa_len to be correctly initialized.

- sa_len is not generally required to be correctly initialized by
  userland in most entry points (with some exceptions, e.g. gif(4)
  configuration).

I don't know whether existing userland programs initialize sa_len
correctly; if they do, we can verify it matches sizeof(struct
sockaddr_at), but there's no need to copy sa_len then if it is
verified to be that fixed value.

Reported-by: syzbot+fb4e112846e31bc4c09d%syzkaller.appspotmail.com@localhost
https://syzkaller.appspot.com/bug?id=6e511f97443d681b8917c6258d6463b28b36e8c9

Reported-by: syzbot+dce8a2b69dc06d9d6115%syzkaller.appspotmail.com@localhost
https://syzkaller.appspot.com/bug?id=e521a01ced5d0bbd363e998b81bc29f3282fd75f

Reported-by: syzbot+705a2a35990549b12c8d%syzkaller.appspotmail.com@localhost
https://syzkaller.appspot.com/bug?id=b9147d4070e1d240acd27155ce1f863997175517

Reported-by: syzbot+b9c649832cc9b1fdc7d6%syzkaller.appspotmail.com@localhost
https://syzkaller.appspot.com/bug?id=d5c9ad3d43042ec6e9f7bd30e13a37205e7e0373

Reported-by: syzbot+ac4ab136ca76e8152218%syzkaller.appspotmail.com@localhost
https://syzkaller.appspot.com/bug?id=349c0c4eded3eff1ff5fc2cd2d88b42806e08a47

Reported-by: syzbot+b75c731d20e46e9543e9%syzkaller.appspotmail.com@localhost
https://syzkaller.appspot.com/bug?id=e014d790e218797e217dd590ebe5a3a89e65b627

XXX pullup-8
XXX pullup-9
XXX pullup-10

diffstat:

 sys/netatalk/at_control.c |  6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diffs (27 lines):

diff -r 000831503e0a -r 9c2609380d31 sys/netatalk/at_control.c
--- a/sys/netatalk/at_control.c Thu Mar 30 15:03:35 2023 +0000
+++ b/sys/netatalk/at_control.c Thu Mar 30 15:58:10 2023 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: at_control.c,v 1.43 2023/03/30 11:21:08 riastradh Exp $         */
+/*     $NetBSD: at_control.c,v 1.44 2023/03/30 15:58:10 riastradh Exp $         */
 
 /*
  * Copyright (c) 1990,1994 Regents of The University of Michigan.
@@ -27,7 +27,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: at_control.c,v 1.43 2023/03/30 11:21:08 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: at_control.c,v 1.44 2023/03/30 15:58:10 riastradh Exp $");
 
 #include "opt_atalk.h"
 
@@ -421,7 +421,7 @@ at_ifinit(struct ifnet *ifp, struct at_i
          * that phase 1 probes use only nodes, not net.node pairs.  Under
          * phase 2, both the net and node must be the same.
          */
-       AA_SAT(aa)->sat_len = sat->sat_len;
+       AA_SAT(aa)->sat_len = sizeof(struct sockaddr_at);
        AA_SAT(aa)->sat_family = AF_APPLETALK;
        if (ifp->if_flags & IFF_LOOPBACK) {
                AA_SAT(aa)->sat_addr.s_net = sat->sat_addr.s_net;