Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 2 Dec 2022 12:53:44 -0800
From:   Andrew Morton <akpm@linux-foundation.org>
To:     Aditya Garg <gargaditya08@live.com>
Cc:     "willy@infradead.org" <willy@infradead.org>,
        "ira.weiny@intel.com" <ira.weiny@intel.com>,
        "axboe@kernel.dk" <axboe@kernel.dk>,
        "bvanassche@acm.org" <bvanassche@acm.org>,
        "keescook@chromium.org" <keescook@chromium.org>,
        "songmuchun@bytedance.com" <songmuchun@bytedance.com>,
        "slava@dubeyko.com" <slava@dubeyko.com>,
        "linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Christoph Hellwig <hch@lst.de>
Subject: Re: [PATCH] hfsplus: Add module parameter to enable force writes
Message-Id: <20221202125344.4254ab20d2fe0a8e784b33e8@linux-foundation.org>
In-Reply-To: <53821C76-DAFE-4505-9EC8-BE4ACBEA9DD9@live.com>
References: <53821C76-DAFE-4505-9EC8-BE4ACBEA9DD9@live.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Fri, 2 Dec 2022 06:01:16 +0000 Aditya Garg <gargaditya08@live.com> wrote:

> From: Aditya Garg <gargaditya08@live.com>
> 
> This patch enables users to permanently enable writes of HFS+ locked
> and/or journaled volumes using a module parameter.
> 
> Why module parameter?
> Reason being, its not convenient to manually mount the volume with force
> everytime. There are use cases which are fine with force enabling writes
> on journaled volumes. I've seen many on various online forums and I am one
> of them as well.
> 
> Isn't it risky?
> Yes obviously it is, as the driver itself warns users for the same. But
> any user using the parameter obviously shall be well aware of the risks
> involved. To be honest, I've been writing on a 100Gb journaled volume for
> a few days, including both large and small files, and haven't faced any
> corruption yet.
> 

Presumably anyone who enables this knows the risk, and if it's a
convenience, why not.

Documentation/filesystems/hfsplus.rst would be a good place to document
this module parameter please.

> --- a/fs/hfsplus/super.c
> +++ b/fs/hfsplus/super.c
> @@ -459,12 +477,20 @@ static int hfsplus_fill_super(struct super_block *sb, void *data, int silent)
>  	} else if (test_and_clear_bit(HFSPLUS_SB_FORCE, &sbi->flags)) {
>  		/* nothing */
>  	} else if (vhdr->attributes & cpu_to_be32(HFSPLUS_VOL_SOFTLOCK)) {
> -		pr_warn("Filesystem is marked locked, mounting read-only.\n");
> -		sb->s_flags |= SB_RDONLY;
> +		if (force_locked_rw) {
> +			pr_warn("Filesystem is marked locked, but writes have been force enabled.\n");
> +		} else {
> +			pr_warn("Filesystem is marked locked, mounting read-only.\n");
> +			sb->s_flags |= SB_RDONLY;
> +		}
>  	} else if ((vhdr->attributes & cpu_to_be32(HFSPLUS_VOL_JOURNALED)) &&
>  			!sb_rdonly(sb)) {
> -		pr_warn("write access to a journaled filesystem is not supported, use the force option at your own risk, mounting read-only.\n");
> -		sb->s_flags |= SB_RDONLY;
> +		if (force_journaled_rw) {
> +			pr_warn("write access to a journaled filesystem is not supported, but has been force enabled.\n");
> +		} else {
> +			pr_warn("write access to a journaled filesystem is not supported, use the force option at your own risk, mounting read-only.\n");
> +			sb->s_flags |= SB_RDONLY;
> +		}

All these super long lines are an eyesore.  How about

			pr_warn("write access to a journaled filesystem is "
				"not supported, but has been force enabled.\n");