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

57

Why this has to be global?

66

I guess we don't need foked and fdp.

128

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

usr.bin/wall/wall.c

105

No need for extra line.

173

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

123

Why we are entering capability mode here?

128

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

58

67

We can combine 3 int variables to a single line.

67–68

Why we need ./ ?

93

So WRONLY or RDONLY or maybe O_RDWR?

97

127–128

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

72

78

81

Why?

144

205

[hanslu952_gmail.com]

usr.bin/wall/ttymsg.c

127–128

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–128

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

[hanslu952_gmail.com]

usr.bin/wall/ttymsg.c

67–68

to get the needed file without using absolute path

[oshogbo]

usr.bin/wall/ttymsg.c

67–68

But file and ./file are both relative.

[hanslu952_gmail.com]

• add back forked and fix format

[oshogbo]

usr.bin/wall/ttymsg.c

66

Why fd2? Where is fd1?

128

Why close dfd?

132

Why dfd?
Where do we clean fd2?

140–144

Why dfd?

152–153

Why dfd?

usr.bin/wall/wall.c

76

Why fd2? Where is fd1?

81

Why we need these transformations?

172

Wasn't devfd was closed in ttymsg?

[hanslu952_gmail.com]

• fix fd problem

[oshogbo]

usr.bin/wall/ttymsg.c

87

With got we cannot get these errors?

87

sorry...

With "openat" we cannot get these errors?

94

Why have we changed the way we return errors?

130

Why have you changed identation?

131

Where are we closing the process descriptor?

136

Why we removed this?

159

Why we remove this part?

usr.bin/wall/wall.c

101

I guess /dev/pts?

[hanslu952_gmail.com]

usr.bin/wall/ttymsg.c

131

I think _exit() will close the process?

[oshogbo]

usr.bin/wall/ttymsg.c

131

Which one?
Which one has a process descriptor?

[hanslu952_gmail.com]

usr.bin/wall/ttymsg.c

131

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

[hanslu952_gmail.com]

usr.bin/wall/ttymsg.c

131

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