linux-doc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH 1/2] scripts/get_abi: fix source path leak
@ 2023-12-31 23:59 Vegard Nossum
  2023-12-31 23:59 ` [RFC PATCH 2/2] docs: kernel_abi.py: fix command injection Vegard Nossum
  2024-01-03 21:02 ` [RFC PATCH 1/2] scripts/get_abi: fix source path leak Jonathan Corbet
  0 siblings, 2 replies; 3+ messages in thread
From: Vegard Nossum @ 2023-12-31 23:59 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Jonathan Corbet
  Cc: linux-doc, Vegard Nossum, Jani Nikula

The code currently leaks the absolute path of the ABI files into the
rendered documentation.

There exists code to prevent this, but it is not effective when an
absolute path is passed, which it is when $srctree is used.

I consider this to be a minimal, stop-gap fix; a better fix would strip
off the actual prefix instead of hacking it off with a regex.

Link: https://mastodon.social/@vegard/111677490643495163
Cc: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
---
 scripts/get_abi.pl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/get_abi.pl b/scripts/get_abi.pl
index 0ffd5531242a..408bfd0216da 100755
--- a/scripts/get_abi.pl
+++ b/scripts/get_abi.pl
@@ -98,7 +98,7 @@ sub parse_abi {
 	$name =~ s,.*/,,;
 
 	my $fn = $file;
-	$fn =~ s,Documentation/ABI/,,;
+	$fn =~ s,.*Documentation/ABI/,,;
 
 	my $nametag = "File $fn";
 	$data{$nametag}->{what} = "File $name";
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [RFC PATCH 2/2] docs: kernel_abi.py: fix command injection
  2023-12-31 23:59 [RFC PATCH 1/2] scripts/get_abi: fix source path leak Vegard Nossum
@ 2023-12-31 23:59 ` Vegard Nossum
  2024-01-03 21:02 ` [RFC PATCH 1/2] scripts/get_abi: fix source path leak Jonathan Corbet
  1 sibling, 0 replies; 3+ messages in thread
From: Vegard Nossum @ 2023-12-31 23:59 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Jonathan Corbet
  Cc: linux-doc, Vegard Nossum, Jani Nikula

The kernel-abi directive passes its argument straight to the shell.
This is unfortunate and unnecessary.

Let's always use paths relative to $srctree/Documentation/ and use
subprocess.check_call() instead of subprocess.Popen(shell=True).

This also makes the code shorter.

Link: https://fosstodon.org/@jani/111676532203641247
Reported-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
---
 Documentation/admin-guide/abi-obsolete.rst |  2 +-
 Documentation/admin-guide/abi-removed.rst  |  2 +-
 Documentation/admin-guide/abi-stable.rst   |  2 +-
 Documentation/admin-guide/abi-testing.rst  |  2 +-
 Documentation/sphinx/kernel_abi.py         | 56 ++++------------------
 5 files changed, 14 insertions(+), 50 deletions(-)

diff --git a/Documentation/admin-guide/abi-obsolete.rst b/Documentation/admin-guide/abi-obsolete.rst
index d095867899c5..594e697aa1b2 100644
--- a/Documentation/admin-guide/abi-obsolete.rst
+++ b/Documentation/admin-guide/abi-obsolete.rst
@@ -7,5 +7,5 @@ marked to be removed at some later point in time.
 The description of the interface will document the reason why it is
 obsolete and when it can be expected to be removed.
 
-.. kernel-abi:: $srctree/Documentation/ABI/obsolete
+.. kernel-abi:: ABI/obsolete
    :rst:
diff --git a/Documentation/admin-guide/abi-removed.rst b/Documentation/admin-guide/abi-removed.rst
index f7e9e43023c1..f9e000c81828 100644
--- a/Documentation/admin-guide/abi-removed.rst
+++ b/Documentation/admin-guide/abi-removed.rst
@@ -1,5 +1,5 @@
 ABI removed symbols
 ===================
 
-.. kernel-abi:: $srctree/Documentation/ABI/removed
+.. kernel-abi:: ABI/removed
    :rst:
diff --git a/Documentation/admin-guide/abi-stable.rst b/Documentation/admin-guide/abi-stable.rst
index 70490736e0d3..fc3361d847b1 100644
--- a/Documentation/admin-guide/abi-stable.rst
+++ b/Documentation/admin-guide/abi-stable.rst
@@ -10,5 +10,5 @@ for at least 2 years.
 Most interfaces (like syscalls) are expected to never change and always
 be available.
 
-.. kernel-abi:: $srctree/Documentation/ABI/stable
+.. kernel-abi:: ABI/stable
    :rst:
diff --git a/Documentation/admin-guide/abi-testing.rst b/Documentation/admin-guide/abi-testing.rst
index b205b16a72d0..19767926b344 100644
--- a/Documentation/admin-guide/abi-testing.rst
+++ b/Documentation/admin-guide/abi-testing.rst
@@ -16,5 +16,5 @@ Programs that use these interfaces are strongly encouraged to add their
 name to the description of these interfaces, so that the kernel
 developers can easily notify them if any changes occur.
 
-.. kernel-abi:: $srctree/Documentation/ABI/testing
+.. kernel-abi:: ABI/testing
    :rst:
diff --git a/Documentation/sphinx/kernel_abi.py b/Documentation/sphinx/kernel_abi.py
index 49797c55479c..5911bd0d7965 100644
--- a/Documentation/sphinx/kernel_abi.py
+++ b/Documentation/sphinx/kernel_abi.py
@@ -39,8 +39,6 @@ import sys
 import re
 import kernellog
 
-from os import path
-
 from docutils import nodes, statemachine
 from docutils.statemachine import ViewList
 from docutils.parsers.rst import directives, Directive
@@ -73,60 +71,26 @@ class KernelCmd(Directive):
     }
 
     def run(self):
-
         doc = self.state.document
         if not doc.settings.file_insertion_enabled:
             raise self.warning("docutils: file insertion disabled")
 
-        env = doc.settings.env
-        cwd = path.dirname(doc.current_source)
-        cmd = "get_abi.pl rest --enable-lineno --dir "
-        cmd += self.arguments[0]
-
-        if 'rst' in self.options:
-            cmd += " --rst-source"
+        srctree = os.path.abspath(os.environ["srctree"])
 
-        srctree = path.abspath(os.environ["srctree"])
+        args = [
+            os.path.join(srctree, 'scripts/get_abi.pl'),
+            'rest',
+            '--enable-lineno',
+            '--dir', os.path.join(srctree, 'Documentation', self.arguments[0]),
+        ]
 
-        fname = cmd
-
-        # extend PATH with $(srctree)/scripts
-        path_env = os.pathsep.join([
-            srctree + os.sep + "scripts",
-            os.environ["PATH"]
-        ])
-        shell_env = os.environ.copy()
-        shell_env["PATH"]    = path_env
-        shell_env["srctree"] = srctree
+        if 'rst' in self.options:
+            args.append('--rst-source')
 
-        lines = self.runCmd(cmd, shell=True, cwd=cwd, env=shell_env)
+        lines = subprocess.check_output(args, cwd=os.path.dirname(doc.current_source)).decode('utf-8')
         nodeList = self.nestedParse(lines, self.arguments[0])
         return nodeList
 
-    def runCmd(self, cmd, **kwargs):
-        u"""Run command ``cmd`` and return its stdout as unicode."""
-
-        try:
-            proc = subprocess.Popen(
-                cmd
-                , stdout = subprocess.PIPE
-                , stderr = subprocess.PIPE
-                , **kwargs
-            )
-            out, err = proc.communicate()
-
-            out, err = codecs.decode(out, 'utf-8'), codecs.decode(err, 'utf-8')
-
-            if proc.returncode != 0:
-                raise self.severe(
-                    u"command '%s' failed with return code %d"
-                    % (cmd, proc.returncode)
-                )
-        except OSError as exc:
-            raise self.severe(u"problems with '%s' directive: %s."
-                              % (self.name, ErrorString(exc)))
-        return out
-
     def nestedParse(self, lines, fname):
         env = self.state.document.settings.env
         content = ViewList()
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [RFC PATCH 1/2] scripts/get_abi: fix source path leak
  2023-12-31 23:59 [RFC PATCH 1/2] scripts/get_abi: fix source path leak Vegard Nossum
  2023-12-31 23:59 ` [RFC PATCH 2/2] docs: kernel_abi.py: fix command injection Vegard Nossum
@ 2024-01-03 21:02 ` Jonathan Corbet
  1 sibling, 0 replies; 3+ messages in thread
From: Jonathan Corbet @ 2024-01-03 21:02 UTC (permalink / raw)
  To: Vegard Nossum, Mauro Carvalho Chehab
  Cc: linux-doc, Vegard Nossum, Jani Nikula

Vegard Nossum <vegard.nossum@oracle.com> writes:

> The code currently leaks the absolute path of the ABI files into the
> rendered documentation.
>
> There exists code to prevent this, but it is not effective when an
> absolute path is passed, which it is when $srctree is used.
>
> I consider this to be a minimal, stop-gap fix; a better fix would strip
> off the actual prefix instead of hacking it off with a regex.
>
> Link: https://mastodon.social/@vegard/111677490643495163
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>

This definitely is worth fixing; I'm a bit annoyed that I let these
problems through.  I've tossed in a Cc: stable as well, backporting them
can only do good.

Thanks,

jon

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2024-01-03 21:02 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-31 23:59 [RFC PATCH 1/2] scripts/get_abi: fix source path leak Vegard Nossum
2023-12-31 23:59 ` [RFC PATCH 2/2] docs: kernel_abi.py: fix command injection Vegard Nossum
2024-01-03 21:02 ` [RFC PATCH 1/2] scripts/get_abi: fix source path leak Jonathan Corbet

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).