hi, back again ;-)
this github page <https://github.com/patatetom/lvm-on-readonly-block-device/blob/main/README.md> will perhaps allow you to better understand my problem and to guide me towards a solution. my main problem is my inability to patch the kernel as I used to do in order to prevent LVM to modify a read-only disk. regards, lacsaP. Le lun. 20 mars 2023 à 10:11, lacsaP Patatetom <[email protected]> a écrit : > hi Emil, > > forgive me if this is the third time I've replied to this message but I > received a moderation warning the two previous times that my reply was over > 40Kb when my preparation file was only 13Kb... > > I haven't had any feedback about this moderation since last week, so I > don't know if you received my answer... > > this one concluded that the recent version of LVM integrated in the ISO > image of ArchLinux modifies a disk, even if this one and all its partitions > are set as read-only (chmod 444 /dev/sdX* && blockdev --setro /dev/sdX* > with udev rule). > > thank you for this investigation, this history. > > regards; lacsaP. > > > Le mer. 15 mars 2023 à 16:36, Emil Velikov <[email protected]> a > écrit : > >> Greetings Pascal, >> >> After following the links I see what's happening. Essentially: >> - Kernel gained RO/RW correctness check - circa Jan 2018, kernel >> commit 721c7fc701c71f693307d274d2b346a1ecd4a534 >> - LVM was initially buggy but later fixed, circa Mar 2018, >> - Kernel check got partially reverted because broken LVM is still >> used - circa Aug 2018, kernel commit >> a32e236eb93e62a0f692e79b7c3c9636689559b9 >> - People used an out of tree patch, reinstating the correctness check >> - The function return type was dropped since it is unused - Sep 2022, >> kernel commit bdb7d420c6f6d2618d4c907cd7742c3195c425e2 >> - kernel patch no longer applies, correct behaviour cannot be enforced >> >> To unblock yourself, it will be a matter of reverting >> bdb7d420c6f6d2618d4c907cd7742c3195c425e2 and then >> a32e236eb93e62a0f692e79b7c3c9636689559b9. >> >> For the mid/long run, one should consider a proper upstream solution: >> >> Assuming I'm in your position, I would dig through the data in the >> linked commits and estimate which/how many distributions ship with >> buggy LVM. Then formulate a comprehensive cover letter, proposing a) >> reverts (if LVM is no longer used in the wild) or b) reverts && a >> (KCONFIG, sysctl, other) toggle to control the behaviour. >> >> Hope that helps, >> Emil >> >> On Wed, 15 Mar 2023 at 13:38, Pascal <[email protected]> wrote: >> > >> > hi Emil, >> > >> > in view of your answer and after rereading my email, I realize that I >> was confused in my request. >> > here it is, I hope, more clearly reformulated :-) >> > >> > first of all, I use ArchLinux to, from time to time, compile the >> slightly modified LTS kernel, and this from PKGBUILD provided by ArchLinux >> at some point. >> > >> > some technologies such as LVM do not take into account the read-only >> applied on a block device. >> > see the two links provided in the previous exchanges for more details... >> > >> > >> > until now, I recompiled the kernel by applying a slight modification to >> the bio_check_ro function present in the blk-core.c source file. >> > the last time I made this modification was on the Linux-LTS-5.10.19 >> kernel : >> > ( >> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/block/blk-core.c?h=v5.10.19 >> ) >> > >> > $ diff -u 5.10.19.original/blk-core.c 5.10.19.me/blk-core.c >> > --- 5.10.19.original/blk-core.c 2023-03-15 13:44:20.176929833 +0100 >> > +++ 5.10.19.me/blk-core.c 2023-03-15 13:44:02.353596114 +0100 >> > @@ -706,7 +706,7 @@ >> > "Trying to write to read-only block-device %s (partno %d)\n", >> > bio_devname(bio, b), part->partno); >> > /* Older lvm-tools actually trigger this */ >> > - return false; >> > + return true; >> > } >> > >> > return false; >> > >> > the compilation of the modified LTS 5.10.19 kernel went well and the >> correction seems to do the job... >> > >> > >> > since this last time (2022/01), the source file blk-core.c has been >> modified a lot and the bio_check_ro function is part of these modifications >> : >> > ( >> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/block/blk-core.c?h=v6.1.15 >> ) >> > >> > $ diff -u 5.10.19.original/blk-core.c 6.1.15.original/blk-core.c >> > --- 5.10.19.original/blk-core.c 2023-03-15 13:44:20.176929833 +0100 >> > +++ 6.1.15.original/blk-core.c 2023-03-15 13:50:36.560271323 +0100 >> > @@ -14,11 +14,10 @@ >> > */ >> > #include <linux/kernel.h> >> > #include <linux/module.h> >> > -#include <linux/backing-dev.h> >> > #include <linux/bio.h> >> > #include <linux/blkdev.h> >> > -#include <linux/blk-mq.h> >> > ... >> > @@ -681,40 +483,22 @@ >> > } >> > >> > late_initcall(fail_make_request_debugfs); >> > - >> > -#else /* CONFIG_FAIL_MAKE_REQUEST */ >> > - >> > -static inline bool should_fail_request(struct hd_struct *part, >> > - unsigned int bytes) >> > -{ >> > - return false; >> > -} >> > - >> > #endif /* CONFIG_FAIL_MAKE_REQUEST */ >> > >> > -static inline bool bio_check_ro(struct bio *bio, struct hd_struct >> *part) >> > +static inline void bio_check_ro(struct bio *bio) >> > { >> > - const int op = bio_op(bio); >> > - >> > - if (part->policy && op_is_write(op)) { >> > - char b[BDEVNAME_SIZE]; >> > - >> > + if (op_is_write(bio_op(bio)) && bdev_read_only(bio->bi_bdev)) { >> > if (op_is_flush(bio->bi_opf) && !bio_sectors(bio)) >> > - return false; >> > - >> > - WARN_ONCE(1, >> > - "Trying to write to read-only block-device %s (partno >> %d)\n", >> > - bio_devname(bio, b), part->partno); >> > + return; >> > + pr_warn("Trying to write to read-only block-device %pg\n", >> > + bio->bi_bdev); >> > /* Older lvm-tools actually trigger this */ >> > - return false; >> > } >> > - >> > - return false; >> > } >> > ... >> > >> > >> > when I introduce my little modification (see diff below) in the code, >> makepkg fails to compile with the error "return with a value in function >> returning void" (see makepkg below) : >> > >> > $ diff -u 6.1.15.original/blk-core.c 6.1.15.me/blk-core.c >> > --- 6.1.15.original/blk-core.c 2023-03-15 13:50:36.560271323 +0100 >> > +++ 6.1.15.me/blk-core.c 2023-03-15 13:56:15.246945330 +0100 >> > @@ -493,6 +493,7 @@ >> > pr_warn("Trying to write to read-only block-device %pg\n", >> > bio->bi_bdev); >> > /* Older lvm-tools actually trigger this */ >> > + return true; >> > } >> > } >> > >> > $ makepkg >> > ... >> > CC block/blk-core.o >> > block/blk-core.c: In function 'bio_check_ro': >> > block/blk-core.c:496:24: error: 'return' with a value, in function >> returning void [-Werror=return-type] >> > 496 | return true; >> > | ^~~~ >> > block/blk-core.c:488:20: note: declared here >> > 488 | static inline void bio_check_ro(struct bio *bio) >> > | ^~~~~~~~~~~~ >> > cc1: some warnings being treated as errors >> > make[2]: *** [scripts/Makefile.build:250: block/blk-core.o] Error 1 >> > make[1]: *** [scripts/Makefile.build:500: block] Error 2 >> > make: *** [Makefile:2005: .] Error 2 >> > >> > >> > so, how to modify the current code of the bio_check_ro function to get >> the desired result (eg. writes KO on RO blockdevice) ? >> > with the changes in the blk-core.c source code since version 5.10.19, >> is it still necessary to tweak the bio_check_ro function to disallow >> technologies that ignore the read-only block? >> > >> > regards, lacsaP. >> > >> > Le mer. 15 mars 2023 à 12:37, Emil Velikov <[email protected]> >> a écrit : >> >> >> >> Greetings Pascal, >> >> >> >> Couple of suggestions from the peanut gallery, Take them with a heavy >> >> pinch of salt: >> >> - Is the issue happening with upstream code from kernel.org? >> >> - Consider mentioning the commit sha (and URL, if it is missing from >> >> kernel.org) in the email >> >> - Is "intervened" the right word here - the Cambridge dictionary >> >> defines it as "to intentionally become involved in a difficult >> >> situation in order to improve it or prevent it from getting worse" >> >> - Are you contacting a developer only? Have you considered adding the >> >> subsystem maintainer and mailing list in the CC list - >> >> scripts/get_maintainer.pl will give you those >> >> - Have you considered opening a bug report, or better yet sending a >> >> patch? Patch does not have to be perfect and if you have doubts you >> >> can mention those in the email/cover-letter. >> >> >> >> Hope that helps >> >> Emil >> >> >> >> [1] https://dictionary.cambridge.org/dictionary/english/intervene >> >> >> >> On Wed, 15 Mar 2023 at 08:42, Pascal <[email protected]> wrote: >> >> > >> >> > hi, >> >> > >> >> > I come to you for lack of feedback (I think the Linux kernel >> developers have other cats to whip :-)) >> >> > would one of you have the answer or a track to follow concerning the >> question below ? >> >> > the encountered compilation error is behind the forwarded email. >> >> > >> >> > regards, lacsaP. >> >> > >> >> > ---------- Forwarded message --------- >> >> > De : Pascal <[email protected]> >> >> > Date: mer. 8 mars 2023 à 14:09 >> >> > Subject: bio_check_ro @ blk-core.c >> >> > >> >> > hi, >> >> > >> >> > I'm addressing you because you intervened (commit) in the function >> bio_check_ro @ blk-core.c @ Linux-LTS-6.1.15. >> >> > the last time I intervened on this file (@ Linux-LTS-5.10.19 for >> personal use), it was to replace "return false;" by "return true;", which >> theoretically should prevent the possible writing on a device locked in >> read-only mode (see here or here). >> >> > with @ Linux-LTS-6.1.15, if I insert "return true;", I now have a >> compilation error. >> >> > in your opinion, is there still a need to "fix" blk-core.c to >> prevent writing to a read-only locked device and if so, can you help me >> implement this fix? >> >> > >> >> > regards, lacsaP. >> >> > ---------- End forwarded message --------- >> >> > >> >> > SYNC include/config/auto.conf >> >> > CC arch/x86/kernel/asm-offsets.s >> >> > CALL scripts/checksyscalls.sh >> >> > DESCEND objtool >> >> > DESCEND bpf/resolve_btfids >> >> > CC block/bdev.o >> >> > CC block/fops.o >> >> > CC block/bio.o >> >> > CC block/elevator.o >> >> > CC block/blk-core.o >> >> > block/blk-core.c: In function 'bio_check_ro': >> >> > block/blk-core.c:496:24: error: 'return' with a value, in function >> returning void [-Werror=return-type] >> >> > 496 | return true; >> >> > | ^~~~ >> >> > block/blk-core.c:488:20: note: declared here >> >> > 488 | static inline void bio_check_ro(struct bio *bio) >> >> > | ^~~~~~~~~~~~ >> >> > cc1: some warnings being treated as errors >> >> > make[2]: *** [scripts/Makefile.build:250: block/blk-core.o] Error 1 >> >> > make[1]: *** [scripts/Makefile.build:500: block] Error 2 >> >> > make: *** [Makefile:2005: .] Error 2 >> >
