wall(1): cappsicumizing wall

[hanslu952_gmail.com]

Summary

The first problem is to use pdfork.
because fork() is disallowed in capability mode,
we use pdfork() to replace ,the semantic is same as rwhod
The second problem is to limit the file descriptor used by open.
it sets up a specific right (write) for a file descriptor and then
attempts to enforce the limitation. If the operation fails,
the program will terminate with a specific error message

Sponsored by: Google, Inc. (GSoC 2024)

Test Plan

Nonblocking test

setup a c program to fill up the pipe of /dev/pts/$any num
the program code is as follows
https://hackmd.io/@black13524/ryClhYgnA

$ cc -o nonblocking nonblocking.c
$ ./nonblocking
$ make 
$ ktrace ./wall-test msg
$ kdump -f ktrace.out

normal test

$ make 
$ ktrace ./wall-test msg
$ kdump -f ktrace.out

Set up enotcapable to get coredump

$ sysctl kern.trap_enotcap=1
$ sysctl kern.corefile= /tmp/coredumps/%N.core
$ gdb executable corefile

[oshogbo]

In the "Test plan" please provide an example of testing blocking (forked version) and unblocked version.

usr.bin/wall/ttymsg.c

58

Why this has to be global?

66–67

I guess we don't need foked and fdp.

132

Does this code complies? I think you have a typo in variable name.

usr.bin/wall/wall.c

123

No need for extra line.

187

I think we have to enter capability mode a little bit earlier then before exit.
The cap_enter/caph_enter function is a barrier which says here starts a "untrusted part", know you are protecting nothing.

[hanslu952_gmail.com]

• Fix pdfork and take off unneccessary code
• remove line break

[hanslu952_gmail.com]

• fix format of ttymsg

[oshogbo]

Hym I don't think this is complete patch.
Can you try to regenerate git diff -U99999 first_commit^ (the ^ is important)

[hanslu952_gmail.com]

Reupload the patch

[oshogbo]

It still doesn't seem right.

[hanslu952_gmail.com]

Trying on more earlier version

[hanslu952_gmail.com]

Sorry I found it

[lwhsu]

There are many whitespace changes, if possible, please don't change the original ones (even they are wrong) in a feature addition change, or it would cause unnecessary review complexity.

[oshogbo]

usr.bin/wall/ttymsg.c

126

Why we are entering capability mode here?

132

How can this work?
You are reassigning fd (which was the open file) to a process descriptor.

[hanslu952_gmail.com]

This comment was removed by hanslu952_gmail.com.

[hanslu952_gmail.com]

path sandboxing and enter capability mode earlier

• Reupload due to including wrong commit

[hanslu952_gmail.com]

• rearrangement and timecache

[oshogbo]

usr.bin/wall/ttymsg.c

59

67

We can combine 3 int variables to a single line.

69

Why we need ./ ?

96

So WRONLY or RDONLY or maybe O_RDWR?

100

127–131

How removed forked change the process menagment?
How many child process will have this process before and after this change.

usr.bin/wall/wall.c

74

83

Why?

87–88

159

221

[hanslu952_gmail.com]

usr.bin/wall/ttymsg.c

127–131

each parent still creates one child,but the number of parent process grows exponentially when pdfork is called more than once.
like:
once is the same
when pdfork is called twice
we will have five parent ,each parent create one child

[oshogbo]

usr.bin/wall/ttymsg.c

127–131

I don't think this what we want, no?

[hanslu952_gmail.com]

usr.bin/wall/ttymsg.c

69

to get the needed file without using absolute path

[oshogbo]

usr.bin/wall/ttymsg.c

69

But file and ./file are both relative.

[hanslu952_gmail.com]

• add back forked and fix format

[oshogbo]

usr.bin/wall/ttymsg.c

66–67

Why fd2? Where is fd1?

132

Why close dfd?

136

Why dfd?
Where do we clean fd2?

149

Why dfd?

160

Why dfd?

usr.bin/wall/wall.c

78

Why fd2? Where is fd1?

86

Why we need these transformations?

186

Wasn't devfd was closed in ttymsg?

[hanslu952_gmail.com]

• fix fd problem

[oshogbo]

usr.bin/wall/ttymsg.c

86

With got we cannot get these errors?

86

sorry...

With "openat" we cannot get these errors?

97

Why have we changed the way we return errors?

133

Why we removed this?

134

Why have you changed identation?

135

Where are we closing the process descriptor?

159

Why we remove this part?

usr.bin/wall/wall.c

119

I guess /dev/pts?

[hanslu952_gmail.com]

usr.bin/wall/ttymsg.c

135

I think _exit() will close the process?

[oshogbo]

usr.bin/wall/ttymsg.c

135

Which one?
Which one has a process descriptor?

[hanslu952_gmail.com]

usr.bin/wall/ttymsg.c

135

It seems not in tty,in main after closing devfd?

[hanslu952_gmail.com]

usr.bin/wall/ttymsg.c

135

Oh sorry,I used gdb wrong ,It wont exit.I found that it should exit at 160 - 161,so I will add back it

[hanslu952_gmail.com]

• add back forked and fix returned errors