Re: [PATCH 6.6 003/331] docs: kernel_feat.py: fix potential command injection

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Feb 04, 2024 at 06:05:05PM +0100, Salvatore Bonaccorso wrote:
> Hi,
> 
> On Thu, Feb 01, 2024 at 05:34:25PM +0100, Vegard Nossum wrote:
> > 
> > On 01/02/2024 16:07, Justin Forbes wrote:
> > > On Thu, Feb 1, 2024 at 8:58 AM Justin Forbes <jforbes@xxxxxxxxxxxxxxxxx> wrote:
> > > > On Thu, Feb 1, 2024 at 8:41 AM Justin Forbes <jforbes@xxxxxxxxxxxxxxxxx> wrote:
> > > > > On Thu, Feb 1, 2024 at 8:25 AM Greg Kroah-Hartman
> > > > > <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> > > > > > On Thu, Feb 01, 2024 at 06:43:46AM -0600, Justin Forbes wrote:
> > > > > > > On Tue, Jan 30, 2024 at 10:21 AM Jonathan Corbet <corbet@xxxxxxx> wrote:
> > > > > > > > Justin Forbes <jforbes@xxxxxxxxxxxxxxxxx> writes:
> > > > > > > > > On Mon, Jan 29, 2024 at 09:01:07AM -0800, Greg Kroah-Hartman wrote:
> > > > > > > > > > 6.6-stable review patch.  If anyone has any objections, please let me know.
> > > > > > > > > > 
> > > > > > > > > > ------------------
> > > > > > > > > > 
> > > > > > > > > > From: Vegard Nossum <vegard.nossum@xxxxxxxxxx>
> > > > > > > > > > 
> > > > > > > > > > [ Upstream commit c48a7c44a1d02516309015b6134c9bb982e17008 ]
> > > > > > > > > > 
> > > > > > > > > > The kernel-feat directive passes its argument straight to the shell.
> > > > > > > > > > This is unfortunate and unnecessary.
> > 
> > [...]
> > 
> > > > > > > > > This patch seems to be missing something. In 6.6.15-rc1 I get a doc
> > > > > > > > > build failure with:
> > > > > > > > > 
> > > > > > > > > /builddir/build/BUILD/kernel-6.6.14-332-g1ff49073b88b/linux-6.6.15-0.rc1.1ff49073b88b.200.fc39.noarch/Documentation/sphinx/kerneldoc.py:133: SyntaxWarning: invalid escape sequence '\.'
> > > > > > > > >    line_regex = re.compile("^\.\. LINENO ([0-9]+)$")
> > > > > > > > 
> > > > > > > > Ah ... you're missing 86a0adc029d3 (Documentation/sphinx: fix Python
> > > > > > > > string escapes).  That is not a problem with this patch, though; I would
> > > > > > > > expect you to get the same error (with Python 3.12) without.
> > > > > > > 
> > > > > > > Well, it appears that 6.6.15 shipped anyway, with this patch included,
> > > > > > > but not with 86a0adc029d3.  If anyone else builds docs, this thread
> > > > > > > should at least show them the fix.  Perhaps we can get the missing
> > > > > > > patch into 6.6.16?
> > > > > > 
> > > > > > Sure, but again, that should be independent of this change, right?
> > > > > 
> > > > > I am not sure I would say independent. This particular change causes
> > > > > docs to fail the build as I mentioned during rc1.  There were no
> > > > > issues building 6.6.14 or previous releases, and no problem building
> > > > > 6.7.3.
> > > > 
> > > > I can confirm that adding this patch to 6.6.15 makes docs build again.
> > > 
> > > I lied, it just fails slightly differently. Some of the noise is gone,
> > > but we still have:
> > > Sphinx parallel build error:
> > > UnboundLocalError: cannot access local variable 'fname' where it is
> > > not associated with a value
> > > make[2]: *** [Documentation/Makefile:102: htmldocs] Error 2
> > > make[1]: *** [/builddir/build/BUILD/kernel-6.6.15/linux-6.6.15-200.fc39.noarch/Makefile:1715:
> > > htmldocs] Error 2
> > 
> > The old version of the script unconditionally assigned a value to the
> > local variable 'fname' (not a value that makes sense to me, since it's
> > literally assigning the whole command, not just a filename, but that's a
> > separate issue), and I removed that so it's only conditionally assigned.
> > This is almost certainly a bug in my patch.
> > 
> > I'm guessing maybe a different patch between 6.6 and current mainline is
> > causing 'fname' to always get assigned for the newer versions and thus
> > make the run succeed, in spite of the bug.
> > 
> > Something like the patch below (completely untested) should restore the
> > previous behaviour, but I'm not convinced it's correct.
> > 
> > 
> > Vegard
> > 
> > diff --git a/Documentation/sphinx/kernel_feat.py
> > b/Documentation/sphinx/kernel_feat.py
> > index b9df61eb4501..15713be8b657 100644
> > --- a/Documentation/sphinx/kernel_feat.py
> > +++ b/Documentation/sphinx/kernel_feat.py
> > @@ -93,6 +93,8 @@ class KernelFeat(Directive):
> >          if len(self.arguments) > 1:
> >              args.extend(['--arch', self.arguments[1]])
> > 
> > +        fname = ' '.join(args)
> > +
> >          lines = subprocess.check_output(args,
> > cwd=os.path.dirname(doc.current_source)).decode('utf-8')
> > 
> >          line_regex = re.compile(r"^\.\. FILE (\S+)$")
> 
> We have as well a documention build problem in Debian, cf.
> https://buildd.debian.org/status/fetch.php?pkg=linux&arch=all&ver=6.6.15-1&stamp=1707050360&raw=0
> though not yet using python 3.12 as default.
> 
> Your above change seems to workaround the issue in fact, but need to
> do a full build yet.

For Debian I'm temporarily reverting from the 6.6.15 upload:

e961f8c6966a ("docs: kernel_feat.py: fix potential command injection")

This is not the best solution, but unbreaks several other builds.

The alternative would be to apply Vegard's workaround or the proper
solution for that.

Regards,
Salvatore




[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux