On Fri, Jan 22, 2021 at 05:16:58PM +0000, Christoph Hellwig wrote:

On Fri, Jan 22, 2021 at 04:15:29PM +0100, Sascha Hauer wrote:

quoted

This patch introduces the Q_PATH flag to the quotactl cmd argument.
When given, the path given in the special argument to quotactl will
be the mount path where the filesystem is mounted, instead of a path
to the block device.
This is necessary for filesystems which do not have a block device as
backing store. Particularly this is done for upcoming UBIFS support.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

I hate overloading quotactl even more.  Why not add a new quotactl_path
syscall instead?

We can probably do that. Honza, what do you think?

quoted

+static struct super_block *quotactl_sb(dev_t dev, int cmd)
 {
 	struct super_block *sb;
 	bool excl = false, thawed = false;
 
 	if (quotactl_cmd_onoff(cmd)) {
 		excl = true;

@@ -901,12 +887,50 @@ static struct super_block *quotactl_block(const char __user *special, int cmd)
 		goto retry;
 	}
 	return sb;
+}
+
+/*
+ * look up a superblock on which quota ops will be performed
+ * - use the name of a block device to find the superblock thereon
+ */
+static struct super_block *quotactl_block(const char __user *special, int cmd)
+{
+#ifdef CONFIG_BLOCK
+	struct filename *tmp = getname(special);
+	int error;
+	dev_t dev;
 
+	if (IS_ERR(tmp))
+		return ERR_CAST(tmp);
+	error = lookup_bdev(tmp->name, &dev);
+	putname(tmp);
+	if (error)
+		return ERR_PTR(error);
+
+	return quotactl_sb(dev, cmd);
 #else
 	return ERR_PTR(-ENODEV);
 #endif

Normal kernel style would be to keep the ifdef entirely outside the
function.

It has been like this before, so changing this should be done in an
extra patch. Not sure if it's worth it though.

quoted

+static struct super_block *quotactl_path(const char __user *special, int cmd)
+{
+	struct super_block *sb;
+	struct path path;
+	int error;
+
+	error = user_path_at(AT_FDCWD, special, LOOKUP_FOLLOW | LOOKUP_AUTOMOUNT,
+			   &path);

This adds an overly long line.

ok, will change.

quoted

+	if (error)
+		return ERR_PTR(error);
+
+	sb = quotactl_sb(path.mnt->mnt_sb->s_dev, cmd);

I think quotactl_sb should take the superblock directly.  This will
need a little refactoring of user_get_super, but will lead to much
better logic.

What do you mean by "take"? Take the superblock as an argument to
quotactl_sb() or take a reference to the superblock?
Sorry, I don't really get where you aiming at.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |