linprocfs: Add support for proc/sysvipc/{msg,sem,shm} #1218

ricardobranco777 · 2024-05-04T18:34:35Z

Add support for proc/sysvipc/{msg,sem,shm}

Tracking in https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=278812

To build:

cd /sys/modules/linprocfs
make && doas make install && doas cp -f /boot/modules/linprocfs.ko /boot/kernel/
doas kldxref /boot/kernel/
doas umount -a -t linprocfs
doas kldunload -v linprocfs
doas kldload -v linprocfs
doas mount -t linprocfs linprocfs /compat/linux/proc/

How I tested it (needs ipcmk from the util-linux package):

$ ipcmk --shmem 128K
Shared memory id: 131072

$ cat /compat/linux/proc/sysvipc/shm 
       key      shmid perms                  size  cpid  lpid nattch   uid   gid  cuid  cgid      atime      dtime      ctime                   rss                  swap
-264176145     131072  4644                131072 39330     0      0  1000  1000  1000  1000          0          0 1714846976                    -1                    -1

$ ipcmk --semaphore 7 
Semaphore id: 196608

$ cat /compat/linux/proc/sysvipc/sem
       key      semid perms      nsems   uid   gid  cuid  cgid      otime      ctime
1038146433     196608  1644          7  1000  1000  1000  1000          0 1714847009

$ ipcmk --queue
Message queue id: 131072

$ cat /compat/linux/proc/sysvipc/msg
       key      msqid perms      cbytes       qnum lspid lrpid   uid   gid  cuid  cgid      stime      rtime      ctime
 756788372     131072   644           0          0     0     0  1000  1000  1000  1000          0          0 1714847025

$ ipcs -a
Message Queues:
T           ID          KEY MODE        OWNER    GROUP    CREATOR  CGROUP                 CBYTES                 QNUM               QBYTES        LSPID        LRPID STIME    RTIME    CTIME
q       131072    756788372 --rw-r--r-- ricardo  ricardo  ricardo  ricardo             0            0                 2048            0            0 no-entry no-entry 20:23:45

Shared Memory:
T           ID          KEY MODE        OWNER    GROUP    CREATOR  CGROUP         NATTCH        SEGSZ         CPID         LPID ATIME    DTIME    CTIME
m       131072   -264176145 --rw-r--r-- ricardo  ricardo  ricardo  ricardo             0       131072        39330            0 no-entry no-entry 20:22:56

Semaphores:
T           ID          KEY MODE        OWNER    GROUP    CREATOR  CGROUP          NSEMS OTIME    CTIME
s       196608   1038146433 --rw-r--r-- ricardo  ricardo  ricardo  ricardo             7 no-entry 20:23:29

The filtering is the same as done by ipcs:

Format string adapted from:

sys/compat/linprocfs/linprocfs.c

bsdimp

A couple of poorly handled races I think?

In some cases, the USB_QUIRK_VP macro was being misused. Instead of setting quirks to the intended value, the first two supplied quirks would go into lo_rev and hi_rev. Replace it with USB_QUIRK_VO which only takes the needed args. This also makes the Dummy products, which where being used to correctly set vendor only quirks, not necessary. Reviewed by: imp Pull Request: freebsd#1153

Seperate usb quirks that target specific revisions from those that dont. Alot of the quirks dont use lo_rev and hi_rev, so we can abstract the 0x0000, 0xffff into a macro. [[ This commit is a bit more churn than we like. I carefully reviewed each one and they are all good. The end product is better -- imp ]] Reviewed by: imp Pull Request: freebsd#1153

Signed-off-by: Ricardo Branco <rbranco@suse.de> Reviewed by: imp Pull Request: freebsd#1218

bsdimp · 2024-05-12T15:30:16Z

I've had to revert this. There's a race and some issues with it that need to be resolved. Please either reopen with them fixed, or create a new review and reference this one in the first comment.

bsdimp · 2024-05-12T15:30:53Z

sys/compat/linprocfs/linprocfs.c

+			    msqids[id].u.msg_stime,
+			    msqids[id].u.msg_rtime,
+			    msqids[id].u.msg_ctime);
+


These times need to be cast to (intmax_t) and %jd should be used to print them.

bsdimp · 2024-05-12T15:34:13Z

sys/compat/linprocfs/linprocfs.c

+	if (msgmni != msginfo.msgmni) {
+		free(msqids, M_TEMP);
+		goto again;
+	}


Why is msginfo.msgmni changing? It would be better to just reply on the sysctl to get the data. It will return ENOMEM when the data size is too small. The case where it's too big is now handled corrected.

bsdimp · 2024-05-12T15:34:33Z

sys/compat/linprocfs/linprocfs.c

+			    semids[id].u.sem_perm.cgid,
+			    semids[id].u.sem_otime,
+			    semids[id].u.sem_ctime);
+


same as above: intmax_t cast.

bsdimp · 2024-05-12T15:35:30Z

sys/compat/linprocfs/linprocfs.c

+		free(semids, M_TEMP);
+		goto again;
+	}
+


Same as above. I think removing this check and keying off ENOMEM in the sysctl return value is the cleaner way to deal with this since we don't seem to have a cleaner, race-free interface to extract this info.

Same as above. I think removing this check and keying off ENOMEM in the sysctl return value is the cleaner way to deal with this since we don't seem to have a cleaner, race-free interface to extract this info.

What do you mean by "keying off ENOMEM" ?

bsdimp · 2024-05-12T15:35:48Z

sys/compat/linprocfs/linprocfs.c

+		free(shmids, M_TEMP);
+		goto again;
+	}
+


same again race.

bsdimp · 2024-05-12T15:36:01Z

sys/compat/linprocfs/linprocfs.c

+			    shmids[id].u.shm_atime,
+			    shmids[id].u.shm_dtime,
+			    shmids[id].u.shm_ctime,
+			    0, 0);	/* XXX rss & swp are not supported */


same again intmax_t cast.

bsdimp · 2024-05-12T15:36:21Z

sys/compat/linprocfs/linprocfs.c

+		if ((shmids[id].u.shm_perm.mode & SHMSEG_ALLOCATED) != 0)
+			sbuf_printf(sb,
+			    "%10d %10lu  %4o %21lu %5u %5u  %5u %5u %5u %5u %5u %10ld %10ld %10ld %21d %21d\n",
+			    (int) shmids[id].u.shm_perm.key,


the %21lu should be %21zu because this is a size_t.

bsdimp · 2024-05-12T16:04:22Z

Also from kib: "The recheck is more problematic, I do agree. Also, I realized one more
issue there: the data structures like msginfo are from sysvmsg.ko modules.
Then, the change make linprocfs depending on the sysv*.ko, but this is
not directly recorded with MODULE_DEPENDS(). It worked because GENERIC
compiles SysV support statically."

ricardobranco777 · 2024-05-12T20:07:41Z

Also from kib: "The recheck is more problematic, I do agree. Also, I realized one more issue there: the data structures like msginfo are from sysvmsg.ko modules. Then, the change make linprocfs depending on the sysv*.ko, but this is not directly recorded with MODULE_DEPENDS(). It worked because GENERIC compiles SysV support statically."

We already have these at EOF:

MODULE_DEPEND(linprocfs, sysvmsg, 1, 1, 1);
MODULE_DEPEND(linprocfs, sysvsem, 1, 1, 1);
MODULE_DEPEND(linprocfs, sysvshm, 1, 1, 1);

Signed-off-by: Ricardo Branco <rbranco@suse.de> Reviewed by: imp Pull Request: freebsd/freebsd-src#1218

ricardobranco777 force-pushed the linprocfs_sysvipc branch from 9f5497e to 1231ad5 Compare May 4, 2024 18:51

emaste reviewed May 6, 2024

View reviewed changes

sys/compat/linprocfs/linprocfs.c Outdated Show resolved Hide resolved

ricardobranco777 force-pushed the linprocfs_sysvipc branch from 1231ad5 to 394783b Compare May 6, 2024 21:01

bsdimp reviewed May 10, 2024

View reviewed changes

sys/compat/linprocfs/linprocfs.c Show resolved Hide resolved

bsdimp reviewed May 10, 2024

View reviewed changes

sys/compat/linprocfs/linprocfs.c Show resolved Hide resolved

bsdimp reviewed May 10, 2024

View reviewed changes

sys/compat/linprocfs/linprocfs.c Show resolved Hide resolved

bsdimp requested changes May 10, 2024

View reviewed changes

bsdimp added the changes-required label May 10, 2024

ricardobranco777 force-pushed the linprocfs_sysvipc branch from 394783b to 9ab9db6 Compare May 10, 2024 06:15

bsdimp added ready and removed changes-required labels May 10, 2024

VexedUXR and others added 3 commits May 11, 2024 13:32

linprocfs: Add support for proc/sysvipc/{msg,sem,shm}

099a81a

Signed-off-by: Ricardo Branco <rbranco@suse.de> Reviewed by: imp Pull Request: freebsd#1218

bsdimp force-pushed the linprocfs_sysvipc branch from 9ab9db6 to 099a81a Compare May 11, 2024 19:39

freebsd-git merged commit 099a81a into freebsd:main May 11, 2024
7 of 9 checks passed

bsdimp reviewed May 12, 2024

View reviewed changes

sys/compat/linprocfs/linprocfs.c

free(shmids, M_TEMP);

goto again;

}

Copy link

Member

bsdimp May 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same again race.

bsdimp reviewed May 12, 2024

View reviewed changes

ricardobranco777 mentioned this pull request May 12, 2024

linprocfs: Add support for proc/sysvipc/{msg,sem,shm} v2 #1232

Merged

ricardobranco777 deleted the linprocfs_sysvipc branch May 16, 2024 21:48

bsdjhb pushed a commit to bsdjhb/cheribsd that referenced this pull request Aug 28, 2024

linprocfs: Add support for proc/sysvipc/{msg,sem,shm}

a724f00

Signed-off-by: Ricardo Branco <rbranco@suse.de> Reviewed by: imp Pull Request: freebsd/freebsd-src#1218

bsdjhb pushed a commit to bsdjhb/cheribsd that referenced this pull request Aug 29, 2024

linprocfs: Add support for proc/sysvipc/{msg,sem,shm}

ae55575

Signed-off-by: Ricardo Branco <rbranco@suse.de> Reviewed by: imp Pull Request: freebsd/freebsd-src#1218

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

linprocfs: Add support for proc/sysvipc/{msg,sem,shm} #1218

linprocfs: Add support for proc/sysvipc/{msg,sem,shm} #1218

ricardobranco777 commented May 4, 2024 •

edited

Loading

bsdimp left a comment

bsdimp commented May 12, 2024

bsdimp May 12, 2024

bsdimp May 12, 2024

bsdimp May 12, 2024

bsdimp May 12, 2024

ricardobranco777 May 12, 2024

bsdimp May 12, 2024

bsdimp May 12, 2024

bsdimp May 12, 2024

bsdimp commented May 12, 2024

ricardobranco777 commented May 12, 2024

linprocfs: Add support for proc/sysvipc/{msg,sem,shm} #1218

linprocfs: Add support for proc/sysvipc/{msg,sem,shm} #1218

Conversation

ricardobranco777 commented May 4, 2024 • edited Loading

bsdimp left a comment

Choose a reason for hiding this comment

bsdimp commented May 12, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bsdimp commented May 12, 2024

ricardobranco777 commented May 12, 2024

ricardobranco777 commented May 4, 2024 •

edited

Loading