port-arm/58165: dead or buggy branches in vchiq_close

[campbell+netbsd%mumble.net@localhost]

>Number:         58165
>Category:       port-arm
>Synopsis:       dead or buggy branches in vchiq_close
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    port-arm-maintainer
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Wed Apr 17 17:35:02 +0000 2024
>Originator:     Taylor R Campbell
>Release:        current, 10, 9,
>Organization:
The NetBSD Vchiqtion
>Environment:
>Description:
vchiq_fileops is only used if:

(a) the device minor exists in vchiq_open, and
(b) the global vchiq state is initialized according to vchiq_get_state in vchiq_open.

For (a), we should add `.d_cfdriver = &vchiq_cd, .d_devtounit = dev_minor_unit' to vchiq_cdevsw to avoid open/detach races.

Given (b), this branch in vchiq_close is almost certainly dead:

   1216 		VCHIQ_STATE_T *state = vchiq_get_state();
...
   1224 		if (!state) {
   1225 			ret = -EPERM;
   1226 			goto out;
   1227 		}

https://nxr.netbsd.org/xref/src/sys/external/bsd/vchiq/dist/interface/vchiq_arm/vchiq_arm.c?r=1.21#1210

If it's not dead, it's probably a memory leak, because struct fileops::fo_close is final -- it is never given a second chance to close if the first one had some transient error.

There is also another failure branch in vchiq_close that is obviously dead:

   1214 	if (1) {
...
   1332 	}
   1333 	else {
   1334 		vchiq_log_error(vchiq_arm_log_level,
   1335 			"Unknown minor device");
   1336 		ret = -ENXIO;
   1337 	}

I assume this is here for the sake of reducing merge conflicts with an upstream; if not, it should be pruned.
>How-To-Repeat:
code inspection
>Fix:
Yes, please!

1. add `.d_cfdriver = &vchiq_cd, .d_devtounit = dev_minor_unit' to vchiq_cdevsw
2. prune dead branches in vchiq_close