hi,
back again ;-)
this github page 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 <patatetom@xxxxxxxxx> 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 <emil.l.velikov@xxxxxxxxx> 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 <patatetom@xxxxxxxxx> 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 <emil.l.velikov@xxxxxxxxx> 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 <patatetom@xxxxxxxxx> 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 <patatetom@xxxxxxxxx>
>> > 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
