Em Thu, 06 Oct 2016 11:42:14 +0300 Jani Nikula <jani.nikula@xxxxxxxxx> escreveu: > On Thu, 06 Oct 2016, Markus Heiser <markus.heiser@xxxxxxxxxxx> wrote: > > with this series a reST-directive kernel-cmd is introduced. The kernel-cmd > > directive includes contend from the stdout of a command-line (@mchehab asked > > for). Ok, I ran some tests here and it works as expected. > I like the fact that this removes Documentation/media/Makefile, and > cleans up the Sphinx build rule in Documentation/Makefile.sphinx. Yeah, that sounds great. > Does > this also make the documentation buildable with sphinx-build directly, > without the kernel build system? If so, great. I guess it probably allows that, if you include the extension on some other tree. Just curious here: what use case do you see by building the Kernel documentation without the Kernel tree? > However, I would have much preferred the approach I proposed months ago, > having the extension itself do specifically what parse-headers.pl does > now. While it may seem generic on the surface, I don't think it's a > clean or a secure approach to allow running of arbitrary scripts from > PATH while building documentation. It's certainly not an approach that > should be encouraged. Sorry, but I disagree. The security threat of having a random command doing something wrong is the same as we already have with the Kernel Makefiles, as they can also run a random command. All it is needed is to add this to a Makefile: subdir-y += run_some_evil_cmd If we accept the fact that we do need to run commands when running "make", it doesn't really matter if such command is at a makefile, inside a perl/python script or called via some Sphinx directive. In all cases, patches need to be reviewed by the community, to be sure that they won't introduce any vulnerabilities. Btw, with regards to security, a way bigger threat is if someone introduces a vulnerable code inside the Kernel code, as this will affect a lot more systems than a vulnerability at the documentation build process. Yet, if you think security is still a high risk, my suggestion would be to restrict the kernel-cmd script to only run scripts inside trusted places, like Documentation/sphinx. - The real issue here is that Sphinx itself doesn't provide what it is needed to build the Kernel documentation. Some extra scripts are required. Right now, we converted maybe 5% of the documentation to ReST, and we're using running two perl scripts: - kernel-doc - parse-headers.pl We also identified that, if we want to add the MAINTAINERS file to some documentation (or a parsed version of it), we would need an extra script to filter it[1]. I can think on other use cases to run such scripts[2]. What I'm saying is that, we can keep adding a Sphinx-specific extension for every such needs, with will result in code duplication and will make harder to maintain it, or we can use a generic solution like this kernel-cmd extension. IMO, a generic solution is a way better, as it sounds easier to maintain. Regards, Mauro [1] https://git.linuxtv.org/mchehab/experimental.git/commit/?h=lkml-books&id=c8b07684c0278d7f9d0e30f575eb4be3a2da4c3b [2] There's another possible usecase, with I'm not convinced yet whether should be addressed or not. At the media subsystem, we use 8 out-of-tree scripts that update the list of supported USB and PCI media boards, e. g. the files under: Documentation/media/v4l-drivers/*cardlist.rst Such scripts used to be part of the mercurial tree, before we moved the media development to git, like this one: https://linuxtv.org/hg/v4l-dvb/file/3724e93f7af5/v4l/scripts/bttv.pl Currently, I'm using a local fork of the old scripts, and, when I notice a patch for a driver with a cardlist, if I remember, I run the scripts to add to update the cardlists. While it could be kept OOT forever, if moved it to the Kernel tree, the documentation will always reflect the Kernel status, with is, IMHO, a good thing. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
