- User Since
- Oct 8 2015, 5:33 PM (205 w, 6 d)
Sat, Aug 24
Feb 23 2019
the patch definitely looks like a hack, but i'll leave the final judgment on that to thiago.
Jan 24 2019
here too, it should be "ancestor" not "parent" for extra correctness.
amending the code comment is optional, too.
note that for extra correctness it should be "ancestor" not "parent".
Jan 21 2019
i said "3rd line", not "3rd case". it's about the ancestry; see the other change.
Jan 17 2019
the 3rd line is still wrong ...
right, threads, i didn't think of that. i suppose it might make sense to pthread_kill() from within kcrash to freeze the other threads, but that will be non-atomic, and i wonder whether it wouldn't have undesirable interactions with the attached debugger and/or exiting the process.
commit message inverted here as well.
you got the parent-child ordering wrong in the commit message. ^^
Jan 16 2019
will also need to wait for commit message update, like the other change.
mark handled issues as done here as well.
mostly minor issues left.
Jan 15 2019
Jan 14 2019
yay, i finally have "some" time for this. ^^
Jun 3 2018
May 28 2018
May 27 2018
this dependency tree is a mess. please remove deps to abandoned changes or unabandon what you actually still need, and linearize the history.
note that the commit message needs a minor adjustment now.
but so does using raw pointers. as the stl is available here anyway, it seems like the preferable abstraction layer.
why aren't you standardizing on std::string? that's cleaner than raw char pointers.
i know we discussed this before to some degree, but i don't remember the particulars.
did you make sure that this is the only place where SocketAddress is used?
May 6 2018
as i certainly mentioned somewhere else already, this is redundant with putting the socket in a safe place. but fair enough ...
note that there are also unaddressed comments from previous rounds.
still needs update
Mar 4 2018
not sure why; the changes are semantically separate.
that contradicts the comments i added to both reviews.
the idea is that you can do directory-based access controls on file-based sockets, while the abstract namespace has no controls.
otoh, only linux has the abstract namespace, and it supports peer credential verification as well, so this doesn't actually add any security afaict.
arguably, the patch still makes sense from a maintenance perspective, removing a redundant code path.
fwiw, i'd re-order this patch before the other one - it makes for smaller patches to first remove code and then refactor only what's left.
looks good, though technically speaking this still fixes two separate issues.
Feb 27 2018
But i'm not sure if thats feasible.
Feb 3 2018
Dec 3 2017
you *really* should use a whitelist. it's ok if that breaks some 3rdparty extractor; you'll get a bug report which you can properly evaluate.
you could go totally overboard and assign fine-grained syscall capabilities to individual extractors, but i can't really think of legitimate reasons why that would be necessary in this context.
Aug 8 2017
Mar 17 2017
yeah, but without knowing the root cause, your fix may be just wrong, or at least the comment/commit message may be misleading.
first try googling it; maybe there is even a response on stackoverflow (as so often happens).
then try posting it yourself to stackoverflow.
dunno whether such questions are welcome on the linux-kernel list, but if not, there might be other lists where it would fit.
please verify that this is in fact a kernel bug (include ml references), not something that should be expected for some weird reasons (compare the solaris path).
if this is a bug, does it actually affect released versions which are not being "upgraded away" on short notice? i wouldn't want a workaround for a bug that had a very short life span.