[PATCH 0/2] kernel-shark: Be more careful with kshark-su-record

[PATCH 0/2] kernel-shark: Be more careful with kshark-su-record

[Steven Rostedt]

Two issues with kshark-su-record.

1) It just calls kshark-record with no path, which can be an issue
   if pkexec removes PATH variable, or sets it to something
   that kshark-record is not in.

2) Need to set DISPLAY variable, because on some of my machines,
   the kshark-record failed to execute due to pkexec removing the
   DISPLAY variable.


Steven Rostedt (VMware) (2):
      kernel-shark: Execute kshark-record from same dir as kshark-su-record
      kernel-shark: Set the DISPLAY environment for pkexec kshark-record

----
 kernel-shark/bin/kshark-su-record | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

[PATCH 1/2] kernel-shark: Execute kshark-record from same dir as kshark-su-record

[Steven Rostedt]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

Do not trust the PATH environment variable (and in fact that may not even be
available with the pkexec). Instead, use the same path as kshark-su-record
to find kshark-record. They should always be together anyway.

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel-shark/bin/kshark-su-record | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel-shark/bin/kshark-su-record b/kernel-shark/bin/kshark-su-record
index 2477045f1710..7faff1cb3339 100755
--- a/kernel-shark/bin/kshark-su-record
+++ b/kernel-shark/bin/kshark-su-record
@@ -5,4 +5,5 @@ then
     xhost +si:localuser:root &>/dev/null
 fi
 
-pkexec kshark-record -o ${PWD}/trace.dat
+THIS_DIR=`dirname $0`
+pkexec ${THIS_DIR}/kshark-record -o ${PWD}/trace.dat
-- 
2.20.1

[PATCH 2/2] kernel-shark: Set the DISPLAY environment for pkexec kshark-record

[Steven Rostedt]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

When running a fresh install, I hit this:

 ERROR:  Capture process failed:
 Unknown errorqt.qpa.screen: QXcbConnection: Could not connect to display
 Could not connect to any X display.

This was due to pkexec not saving the DISPLAY environment variable. To solve
this, pass that variable as one of the environment variables to pkexec.

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel-shark/bin/kshark-su-record | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel-shark/bin/kshark-su-record b/kernel-shark/bin/kshark-su-record
index 7faff1cb3339..8c9fbd02a351 100755
--- a/kernel-shark/bin/kshark-su-record
+++ b/kernel-shark/bin/kshark-su-record
@@ -6,4 +6,4 @@ then
 fi
 
 THIS_DIR=`dirname $0`
-pkexec ${THIS_DIR}/kshark-record -o ${PWD}/trace.dat
+pkexec env DISPLAY=${DISPLAY} ${THIS_DIR}/kshark-record -o ${PWD}/trace.dat
-- 
2.20.1

Re: [PATCH 2/2] kernel-shark: Set the DISPLAY environment for pkexec kshark-record

[Yordan Karadzhov (VMware)]

On 24.07.19 г. 1:50 ч., Steven Rostedt wrote:
> From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
> 
> When running a fresh install, I hit this:
> 
>   ERROR:  Capture process failed:
>   Unknown errorqt.qpa.screen: QXcbConnection: Could not connect to display
>   Could not connect to any X display.
> 
> This was due to pkexec not saving the DISPLAY environment variable. To solve
> this, pass that variable as one of the environment variables to pkexec.
> 
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> ---
>   kernel-shark/bin/kshark-su-record | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel-shark/bin/kshark-su-record b/kernel-shark/bin/kshark-su-record
> index 7faff1cb3339..8c9fbd02a351 100755
> --- a/kernel-shark/bin/kshark-su-record
> +++ b/kernel-shark/bin/kshark-su-record
> @@ -6,4 +6,4 @@ then
>   fi
>   
>   THIS_DIR=`dirname $0`
> -pkexec ${THIS_DIR}/kshark-record -o ${PWD}/trace.dat
> +pkexec env DISPLAY=${DISPLAY} ${THIS_DIR}/kshark-record -o ${PWD}/trace.dat
> 


Both patches look good to me.
Thanks!
Yordan

Reviewed-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>

Re: [PATCH 1/2] kernel-shark: Execute kshark-record from same dir as kshark-su-record

[Yordan Karadzhov (VMware)]

On 24.07.19 г. 1:50 ч., Steven Rostedt wrote:
> From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
> 
> Do not trust the PATH environment variable (and in fact that may not even be
> available with the pkexec). Instead, use the same path as kshark-su-record
> to find kshark-record. They should always be together anyway.
> 
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> ---
>   kernel-shark/bin/kshark-su-record | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel-shark/bin/kshark-su-record b/kernel-shark/bin/kshark-su-record
> index 2477045f1710..7faff1cb3339 100755
> --- a/kernel-shark/bin/kshark-su-record
> +++ b/kernel-shark/bin/kshark-su-record
> @@ -5,4 +5,5 @@ then
>       xhost +si:localuser:root &>/dev/null
>   fi
>   
> -pkexec kshark-record -o ${PWD}/trace.dat
> +THIS_DIR=`dirname $0`
> +pkexec ${THIS_DIR}/kshark-record -o ${PWD}/trace.dat
> 

I just realized that there is one problem with the patch. With this 
modification, you can use only the installed version of the script (so 
you more or less depend on PATH).

This will work:
  kshark-su-record

This will work as well:
  /usr/local/bin/kshark-su-record

However this will not work:
  kernel-shark/bin/kshark-su-record

because you are trying to start with pkexec an executable from your $HOME

Thanks!
Yordan

Re: [PATCH 1/2] kernel-shark: Execute kshark-record from same dir as kshark-su-record

[Steven Rostedt]

On Thu, 25 Jul 2019 12:01:34 +0300
"Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:


> I just realized that there is one problem with the patch. With this 
> modification, you can use only the installed version of the script (so 
> you more or less depend on PATH).
> 
> This will work:
>   kshark-su-record
> 
> This will work as well:
>   /usr/local/bin/kshark-su-record
> 
> However this will not work:
>   kernel-shark/bin/kshark-su-record
> 
> because you are trying to start with pkexec an executable from your $HOME

What about this patch?

-- Steve

diff --git a/kernel-shark/bin/kshark-su-record b/kernel-shark/bin/kshark-su-record
index 8c9fbd02a351..152d0cdff3e7 100755
--- a/kernel-shark/bin/kshark-su-record
+++ b/kernel-shark/bin/kshark-su-record
@@ -6,4 +6,8 @@ then
 fi
 
 THIS_DIR=`dirname $0`
+if [ "${THIS_DIR#/}" == "${THIS_DIR}" ]
+then
+    THIS_DIR=`pwd`"/$THIS_DIR"
+fi
 pkexec env DISPLAY=${DISPLAY} ${THIS_DIR}/kshark-record -o ${PWD}/trace.dat

Re: [PATCH 1/2] kernel-shark: Execute kshark-record from same dir as kshark-su-record

[Yordan Karadzhov (VMware)]

On 25.07.19 г. 15:45 ч., Steven Rostedt wrote:
> On Thu, 25 Jul 2019 12:01:34 +0300
> "Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:
> 
> 
>> I just realized that there is one problem with the patch. With this
>> modification, you can use only the installed version of the script (so
>> you more or less depend on PATH).
>>
>> This will work:
>>    kshark-su-record
>>
>> This will work as well:
>>    /usr/local/bin/kshark-su-record
>>
>> However this will not work:
>>    kernel-shark/bin/kshark-su-record
>>
>> because you are trying to start with pkexec an executable from your $HOME
> 
> What about this patch?
> 
> -- Steve
> 
> diff --git a/kernel-shark/bin/kshark-su-record b/kernel-shark/bin/kshark-su-record
> index 8c9fbd02a351..152d0cdff3e7 100755
> --- a/kernel-shark/bin/kshark-su-record
> +++ b/kernel-shark/bin/kshark-su-record
> @@ -6,4 +6,8 @@ then
>   fi
>   
>   THIS_DIR=`dirname $0`
> +if [ "${THIS_DIR#/}" == "${THIS_DIR}" ]
> +then
> +    THIS_DIR=`pwd`"/$THIS_DIR"
> +fi
>   pkexec env DISPLAY=${DISPLAY} ${THIS_DIR}/kshark-record -o ${PWD}/trace.dat
> 

I see that this works, however now I am really confused why it works.

I thought that pkexec will start an executable only from the path 
provided in

kernel-shark/org.freedesktop.kshark-record.policy

Thanks!
Yordan

Re: [PATCH 1/2] kernel-shark: Execute kshark-record from same dir as kshark-su-record

[Yordan Karadzhov (VMware)]

On 25.07.19 г. 15:45 ч., Steven Rostedt wrote:
> On Thu, 25 Jul 2019 12:01:34 +0300
> "Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:
> 
> 
>> I just realized that there is one problem with the patch. With this
>> modification, you can use only the installed version of the script (so
>> you more or less depend on PATH).
>>
>> This will work:
>>    kshark-su-record
>>
>> This will work as well:
>>    /usr/local/bin/kshark-su-record
>>
>> However this will not work:
>>    kernel-shark/bin/kshark-su-record
>>
>> because you are trying to start with pkexec an executable from your $HOME
> 
> What about this patch?
> 
> -- Steve
> 
> diff --git a/kernel-shark/bin/kshark-su-record b/kernel-shark/bin/kshark-su-record
> index 8c9fbd02a351..152d0cdff3e7 100755
> --- a/kernel-shark/bin/kshark-su-record
> +++ b/kernel-shark/bin/kshark-su-record
> @@ -6,4 +6,8 @@ then
>   fi
>   
>   THIS_DIR=`dirname $0`
> +if [ "${THIS_DIR#/}" == "${THIS_DIR}" ]
> +then
> +    THIS_DIR=`pwd`"/$THIS_DIR"
> +fi
>   pkexec env DISPLAY=${DISPLAY} ${THIS_DIR}/kshark-record -o ${PWD}/trace.dat
> 

I see that this works, however now I am really confused why it works.

I thought that pkexec will start an executable only from the path 
provided in

kernel-shark/org.freedesktop.kshark-record.policy

Thanks!
Yordan