If the directory passed to the '.. kernel-feat::' directive does not exist or the get_feat.pl script does not find any files to extract features from, Sphinx will report the following error:
Sphinx parallel build error: UnboundLocalError: local variable 'fname' referenced before assignment make[2]: *** [Documentation/Makefile:102: htmldocs] Error 2
This is due to how I changed the script in c48a7c44a1d0 ("docs: kernel_feat.py: fix potential command injection"). Before that, the filename passed along to self.nestedParse() in this case was weirdly just the whole get_feat.pl invocation.
We can fix it by doing what kernel_abi.py does -- just pass self.arguments[0] as 'fname'.
Fixes: c48a7c44a1d0 ("docs: kernel_feat.py: fix potential command injection") Cc: Justin Forbes jforbes@fedoraproject.org Cc: Salvatore Bonaccorso carnil@debian.org Cc: Jani Nikula jani.nikula@intel.com Cc: Mauro Carvalho Chehab mchehab@kernel.org Cc: stable@vger.kernel.org Signed-off-by: Vegard Nossum vegard.nossum@oracle.com --- Documentation/sphinx/kernel_feat.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/sphinx/kernel_feat.py b/Documentation/sphinx/kernel_feat.py index b9df61eb4501..03ace5f01b5c 100644 --- a/Documentation/sphinx/kernel_feat.py +++ b/Documentation/sphinx/kernel_feat.py @@ -109,7 +109,7 @@ class KernelFeat(Directive): else: out_lines += line + "\n"
- nodeList = self.nestedParse(out_lines, fname) + nodeList = self.nestedParse(out_lines, self.arguments[0]) return nodeList
def nestedParse(self, lines, fname):
Em Mon, 5 Feb 2024 18:51:26 +0100 Vegard Nossum vegard.nossum@oracle.com escreveu:
If the directory passed to the '.. kernel-feat::' directive does not exist or the get_feat.pl script does not find any files to extract features from, Sphinx will report the following error:
Sphinx parallel build error: UnboundLocalError: local variable 'fname' referenced before assignment make[2]: *** [Documentation/Makefile:102: htmldocs] Error 2This is due to how I changed the script in c48a7c44a1d0 ("docs: kernel_feat.py: fix potential command injection"). Before that, the filename passed along to self.nestedParse() in this case was weirdly just the whole get_feat.pl invocation.
We can fix it by doing what kernel_abi.py does -- just pass self.arguments[0] as 'fname'.
Fixes: c48a7c44a1d0 ("docs: kernel_feat.py: fix potential command injection") Cc: Justin Forbes jforbes@fedoraproject.org Cc: Salvatore Bonaccorso carnil@debian.org Cc: Jani Nikula jani.nikula@intel.com Cc: Mauro Carvalho Chehab mchehab@kernel.org Cc: stable@vger.kernel.org Signed-off-by: Vegard Nossum vegard.nossum@oracle.com
Reviewed-by: Mauro Carvalho Chehab mchehab@kernel.org
Documentation/sphinx/kernel_feat.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/sphinx/kernel_feat.py b/Documentation/sphinx/kernel_feat.py index b9df61eb4501..03ace5f01b5c 100644 --- a/Documentation/sphinx/kernel_feat.py +++ b/Documentation/sphinx/kernel_feat.py @@ -109,7 +109,7 @@ class KernelFeat(Directive): else: out_lines += line + "\n"
nodeList = self.nestedParse(out_lines, fname)
nodeList = self.nestedParse(out_lines, self.arguments[0]) return nodeListdef nestedParse(self, lines, fname):
Thanks, Mauro
Hi Vegard,
On Mon, Feb 05, 2024 at 06:51:26PM +0100, Vegard Nossum wrote:
If the directory passed to the '.. kernel-feat::' directive does not exist or the get_feat.pl script does not find any files to extract features from, Sphinx will report the following error:
Sphinx parallel build error: UnboundLocalError: local variable 'fname' referenced before assignment make[2]: *** [Documentation/Makefile:102: htmldocs] Error 2This is due to how I changed the script in c48a7c44a1d0 ("docs: kernel_feat.py: fix potential command injection"). Before that, the filename passed along to self.nestedParse() in this case was weirdly just the whole get_feat.pl invocation.
We can fix it by doing what kernel_abi.py does -- just pass self.arguments[0] as 'fname'.
Fixes: c48a7c44a1d0 ("docs: kernel_feat.py: fix potential command injection") Cc: Justin Forbes jforbes@fedoraproject.org Cc: Salvatore Bonaccorso carnil@debian.org Cc: Jani Nikula jani.nikula@intel.com Cc: Mauro Carvalho Chehab mchehab@kernel.org Cc: stable@vger.kernel.org Signed-off-by: Vegard Nossum vegard.nossum@oracle.com
Documentation/sphinx/kernel_feat.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/sphinx/kernel_feat.py b/Documentation/sphinx/kernel_feat.py index b9df61eb4501..03ace5f01b5c 100644 --- a/Documentation/sphinx/kernel_feat.py +++ b/Documentation/sphinx/kernel_feat.py @@ -109,7 +109,7 @@ class KernelFeat(Directive): else: out_lines += line + "\n"
nodeList = self.nestedParse(out_lines, fname)
nodeList = self.nestedParse(out_lines, self.arguments[0]) return nodeListdef nestedParse(self, lines, fname): -- 2.34.1
Thanks for the fix. Tested doc build on top of v6.6.16 and addresses the issue.
Tested-by: Salvatore Bonaccorso carnil@debian.org
Regards, Salvatore
Vegard Nossum vegard.nossum@oracle.com writes:
If the directory passed to the '.. kernel-feat::' directive does not exist or the get_feat.pl script does not find any files to extract features from, Sphinx will report the following error:
Sphinx parallel build error: UnboundLocalError: local variable 'fname' referenced before assignment make[2]: *** [Documentation/Makefile:102: htmldocs] Error 2This is due to how I changed the script in c48a7c44a1d0 ("docs: kernel_feat.py: fix potential command injection"). Before that, the filename passed along to self.nestedParse() in this case was weirdly just the whole get_feat.pl invocation.
We can fix it by doing what kernel_abi.py does -- just pass self.arguments[0] as 'fname'.
Fixes: c48a7c44a1d0 ("docs: kernel_feat.py: fix potential command injection") Cc: Justin Forbes jforbes@fedoraproject.org Cc: Salvatore Bonaccorso carnil@debian.org Cc: Jani Nikula jani.nikula@intel.com Cc: Mauro Carvalho Chehab mchehab@kernel.org Cc: stable@vger.kernel.org Signed-off-by: Vegard Nossum vegard.nossum@oracle.com
Documentation/sphinx/kernel_feat.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/sphinx/kernel_feat.py b/Documentation/sphinx/kernel_feat.py index b9df61eb4501..03ace5f01b5c 100644 --- a/Documentation/sphinx/kernel_feat.py +++ b/Documentation/sphinx/kernel_feat.py @@ -109,7 +109,7 @@ class KernelFeat(Directive): else: out_lines += line + "\n"
nodeList = self.nestedParse(out_lines, fname)
nodeList = self.nestedParse(out_lines, self.arguments[0]) return nodeList
So I can certainly track this through to 6.8, but I feel like I'm missing something:
- If we have never seen a ".. FILE" line, then (as the changelog notes) no files were found to extract feature information from. In that case, why make the self.nestedParse() call at all? Why not just return rather than making a useless call with a random name?
What am I overlooking?
Thanks,
jon
On 06/02/2024 23:53, Jonathan Corbet wrote:
Vegard Nossum vegard.nossum@oracle.com writes:
@@ -109,7 +109,7 @@ class KernelFeat(Directive): else: out_lines += line + "\n"
nodeList = self.nestedParse(out_lines, fname)
nodeList = self.nestedParse(out_lines, self.arguments[0]) return nodeListSo I can certainly track this through to 6.8, but I feel like I'm missing something:
- If we have never seen a ".. FILE" line, then (as the changelog notes) no files were found to extract feature information from. In that case, why make the self.nestedParse() call at all? Why not just return rather than making a useless call with a random name?
What am I overlooking?
Even if we skip the call in the error/empty case, we still need to pass a sensible value here in the other cases -- this value is the file that will be attributed by Sphinx if there is e.g. a reST syntax error in any of the feature files. 'fname' here is basically the last file that happened to be read by get_feat.pl, which is more misleading than self.arguments[0] IMHO.
So basically: Yes, we could skip the call entirely, but we'd still want to make the above change as well; skipping it doesn't change that.
Maybe we should just change it to doc.current_source directly -- my rationale for splitting it up into two patches was that we would have one patch bringing it sort of back to where we were before (having it at least not error out and still be the obvious/minimal fix that can get backported to stable) and then a patch actually improving on that (the next patch in the series).
Does that make sense?
Vegard
Vegard Nossum vegard.nossum@oracle.com writes:
On 06/02/2024 23:53, Jonathan Corbet wrote:
Vegard Nossum vegard.nossum@oracle.com writes:
@@ -109,7 +109,7 @@ class KernelFeat(Directive): else: out_lines += line + "\n"
nodeList = self.nestedParse(out_lines, fname)
nodeList = self.nestedParse(out_lines, self.arguments[0]) return nodeListSo I can certainly track this through to 6.8, but I feel like I'm missing something:
- If we have never seen a ".. FILE" line, then (as the changelog notes) no files were found to extract feature information from. In that case, why make the self.nestedParse() call at all? Why not just return rather than making a useless call with a random name?
What am I overlooking?
Even if we skip the call in the error/empty case, we still need to pass a sensible value here in the other cases -- this value is the file that will be attributed by Sphinx if there is e.g. a reST syntax error in any of the feature files. 'fname' here is basically the last file that happened to be read by get_feat.pl, which is more misleading than self.arguments[0] IMHO.
The purpose is to point the finger at the file that actually contained the error; are you saying that this isn't working?
Sorry if I'm being slow here,
jon
On 07/02/2024 15:42, Jonathan Corbet wrote:
Vegard Nossum vegard.nossum@oracle.com writes:
On 06/02/2024 23:53, Jonathan Corbet wrote:
Vegard Nossum vegard.nossum@oracle.com writes:
@@ -109,7 +109,7 @@ class KernelFeat(Directive): else: out_lines += line + "\n"
nodeList = self.nestedParse(out_lines, fname)
nodeList = self.nestedParse(out_lines, self.arguments[0]) return nodeListSo I can certainly track this through to 6.8, but I feel like I'm missing something:
- If we have never seen a ".. FILE" line, then (as the changelog notes) no files were found to extract feature information from. In that case, why make the self.nestedParse() call at all? Why not just return rather than making a useless call with a random name?
What am I overlooking?
Even if we skip the call in the error/empty case, we still need to pass a sensible value here in the other cases -- this value is the file that will be attributed by Sphinx if there is e.g. a reST syntax error in any of the feature files. 'fname' here is basically the last file that happened to be read by get_feat.pl, which is more misleading than self.arguments[0] IMHO.
The purpose is to point the finger at the file that actually contained the error; are you saying that this isn't working?
For kernel_feat.py: correct, it never did that.
See my longer explanation here:
https://lore.kernel.org/all/d46018a3-3259-4565-9a25-f9b695519f81@oracle.com/
Vegard
Vegard Nossum vegard.nossum@oracle.com writes:
The purpose is to point the finger at the file that actually contained the error; are you saying that this isn't working?
For kernel_feat.py: correct, it never did that.
See my longer explanation here:
https://lore.kernel.org/all/d46018a3-3259-4565-9a25-f9b695519f81@oracle.com/
OK, good enough. I've applied the patch and will send it Linusward after a bit of linux-next time.
Thanks,
jon
linux-stable-mirror@lists.linaro.org
-
Jonathan Corbet -
Mauro Carvalho Chehab -
Salvatore Bonaccorso -
Vegard Nossum