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 09:31:48PM +0100, Salvatore Bonaccorso wrote:
> 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.

What is the "proper" solution here?  Does 6.8-rc3 work?  What are we
missing to be backported here?

thanks,

greg k-h




[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