What problem are you trying to solve?


> On 8 Aug 2024, at 20:44, Mateusz Guzik <mjguzik@gmail.com> wrote:
> 
> On Thu, Aug 08, 2024 at 03:47:27PM +0000, Theo de Raadt wrote:
>> Todd C. Miller <millert@openbsd.org> wrote:
>> 
>>> On Thu, 08 Aug 2024 13:21:54 +0200, Alexander Bluhm wrote:
>>> 
>>>> Independent of that, can we commit the chunk below?  When searching
>>>> for a specific process, there is not need to traverse the list to
>>>> the end.  Also I check for arg >= 0 as this is cosistent with the
>>>> arg < -1 check before.  Then you don't have to think about what is
>>>> special about process 0.  There is no functional difference as
>>>> process 0 has PS_SYSTEM set and is skipped anyway.
>>> 
>>> Yes, this makes sense. OK millert@
>> 
>> No matter what we do, we don't get an atomic view.  Punting the problem
>> to userland sidesteps the difficult problem in the kernel.
> 
> I just realized literally everyone can call into this code and get the
> info, the only restriction is not showing kernel pointers to non-root.
> 
> I would expect only root to be able to dump the entire list, while non-0
> users would only see their own processes. I verified both Linux and
> FreeBSD restrict equivalent functionality in this vein.
> 
> That aside the BYPID case dumps text/cdir/rdir/trace, while BYUID
> duplicates the code except it lacks text (I'm assuming that's not
> intentional).
> 
> If filtering by pid, why even bother walking the entire list looking for
> it when prfind can use pidhash. This would require some refactoring to
> prevent more code duplication.
> 
> So I would instead suggest:
> 1. a routine which can export stuff from a picked process
> 2. a routine which performs the lookup and walk, it would accept a
> callback for filtering (pid or uid)
> 
> then case KERN_FILE_BYPID could be +/-:
> 	if (arg > 0) {
> 		proc = prfind(arg);
> 		error = nicely_named_export_by_proc(proc);
> 		break;
> 	}
> 	....
> 	kern_file_iterator(kern_file_bypid_filter, arg);
> 	break;
> 
> the by uid case would roll with kern_file_iterator(kern_file_byuid_filter, arg);
> 
> I will also note that the entire thing issues repeat copyout calls.
> that's pretty slow due to repeat SMAP-disabelement trips -- it can
> happen in one go per process
> 
> that's on top of what I said concerning the safety of the entire thing,
> see https://marc.info/?l=openbsd-tech&m=172312202128451&w=2
>