Messages in this thread | | | Date | Fri, 28 Jul 2023 17:00:05 -0600 | Subject | Re: [PATCH next 3/3] selftests:connector: Add root check and fix arg error paths to skip | From | Shuah Khan <> |
| |
On 7/28/23 16:40, Anjali Kulkarni wrote: > > >> On Jul 28, 2023, at 3:25 PM, Shuah Khan <skhan@linuxfoundation.org> wrote: >> >> On 7/28/23 15:59, Anjali Kulkarni wrote: >>>> On Jul 28, 2023, at 2:41 PM, Shuah Khan <skhan@linuxfoundation.org> wrote: >>>> >>>> On 7/28/23 15:21, Anjali Kulkarni wrote: >>>>>> On Jul 28, 2023, at 12:44 PM, Shuah Khan <skhan@linuxfoundation.org> wrote: >>>>>> >>>>>> On 7/28/23 13:06, Shuah Khan wrote: >>>>>>> On 7/28/23 12:10, Anjali Kulkarni wrote: >>>>>>>> >>>>>>>> >>>>>>>>> On Jul 28, 2023, at 10:29 AM, Shuah Khan <skhan@linuxfoundation.org> wrote: >>>>>>>>> >>>>>>>>> proc_filter test requires root privileges. Add root privilege check >>>>>>>>> and skip the test. Also fix argument parsing paths to skip in their >>>>>>>>> error legs. >>>>>>>>> >>>>>>>>> Signed-off-by: Shuah Khan <skhan@linuxfoundation.org> >>>>>>>>> --- >>>>>>>>> tools/testing/selftests/connector/proc_filter.c | 9 +++++++-- >>>>>>>>> 1 file changed, 7 insertions(+), 2 deletions(-) >>>>>>>>> >>>>>>>>> diff --git a/tools/testing/selftests/connector/proc_filter.c b/tools/testing/selftests/connector/proc_filter.c >>>>>>>>> index 4fe8c6763fd8..7b2081b98e5c 100644 >>>>>>>>> --- a/tools/testing/selftests/connector/proc_filter.c >>>>>>>>> +++ b/tools/testing/selftests/connector/proc_filter.c >>>>>>>>> @@ -248,7 +248,7 @@ int main(int argc, char *argv[]) >>>>>>>>> >>>>>>>>> if (argc > 2) { >>>>>>>>> printf("Expected 0(assume no-filter) or 1 argument(-f)\n"); >>>>>>>>> - exit(1); >>>>>>>>> + exit(KSFT_SKIP); >>>>>>>>> } >>>>>>>>> >>>>>>>>> if (argc == 2) { >>>>>>>>> @@ -256,10 +256,15 @@ int main(int argc, char *argv[]) >>>>>>>>> filter = 1; >>>>>>>>> } else { >>>>>>>>> printf("Valid option : -f (for filter feature)\n"); >>>>>>>>> - exit(1); >>>>>>>>> + exit(KSFT_SKIP); >>>>>>>>> } >>>>>>>>> } >>>>>>>>> >>>>>>>>> + if (geteuid()) { >>>>>>>>> + printf("Connector test requires root privileges.\n"); >>>>>>>>> + exit(KSFT_SKIP); >>>>>>>>> + } >>>>>>>>> + >>>>>>>> >>>>>>>> I am not sure why you have added this check? proc_filter does not need root privilege to run. >>>>>>>> >>>>>>> It failed for me when I ran it saying it requires root privileges. >>>>>>> I had to run it as root. >>>>>> >>>>>> The following is what I see when I run the test as non-root >>>>>> user: >>>>>> >>>>>> bind failed: Operation not permitted >>>>>> >>>>> Yes, that’s expected on a kernel which does not have the kernel patches submitted with this selftest installed on it. >>>>> So this check for root needs to be removed. >>>> >>>> I will send v2 for this patch without root check. I should have >>>> split the argument error paths and root check anyway. >>>> >>>> However, what is strange is if the test run by root, bind() doesn't fail. >>>> This doesn't make sense to me based on what you said about bind() fails >>>> if kernel doesn't support the new feature. >>>> >>> I didn’t say that - part of the changes introduced by the patches is to remove the root check and add some features on top of existing code. >> >> Okay. So what should happen if a root user runs this test on a kernel >> that doesn't have the kernel patches submitted with this selftest >> installed on it? >> > > It will default to the behavior previous to my changes - that is it will report all events as opposed to a subset of events (which is the new feature added by my change) >
Okay. Sorry I am unable to follow this explanation. This test has just been added in commit 73a29531f45fed6423144057d7a844aae46dad9d
Can you please look at the usage for this test:
- What should happen when kernel without filtering is run as root or non-root - What should happen when kernel with filtering is run as root or non-root
There seems to be difference in behavior of this test depending on user privileges. This should reflect in the message the user sees.
This message "bind failed: Operation not permitted" doesn't tell user anything - add a better message. Also this needs to be a skip and not fail.
I just sent v2 without the root check.
thanks, -- Shuah
| |