From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pg1-f194.google.com ([209.85.215.194]:40159 "EHLO mail-pg1-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728188AbeISGul (ORCPT ); Wed, 19 Sep 2018 02:50:41 -0400 Date: Wed, 19 Sep 2018 10:15:19 +0900 From: Sergey Senozhatsky To: David Howells Cc: Sergey Senozhatsky , Guenter Roeck , viro@zeniv.linux.org.uk, torvalds@linux-foundation.org, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, Steven Rostedt Subject: Re: [PATCH 14/33] vfs: Implement a filesystem superblock creation/configuration context [ver #11] Message-ID: <20180919011519.GB407@jagdpanzerIV> References: <20180918090722.GA463@jagdpanzerIV> <20180911220743.GA13208@roeck-us.net> <20180911174641.GA15149@roeck-us.net> <153313703562.13253.5766498657900728120.stgit@warthog.procyon.org.uk> <153313714181.13253.304098108512966976.stgit@warthog.procyon.org.uk> <27113.1536702746@warthog.procyon.org.uk> <32382.1536707855@warthog.procyon.org.uk> <20180911235403.GA10107@roeck-us.net> <27019.1537288742@warthog.procyon.org.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <27019.1537288742@warthog.procyon.org.uk> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On (09/18/18 17:39), David Howells wrote: [..] > -static int legacy_init_fs_context(struct fs_context *fc, struct dentry *dentry) > +int legacy_init_fs_context(struct fs_context *fc, struct dentry *dentry) > { > + switch (fc->purpose) { > + default: > + fc->fs_private = kzalloc(sizeof(struct legacy_fs_context), > + GFP_KERNEL); > + if (!fc->fs_private) > + return -ENOMEM; ops->reconfigure() invoked for FS_CONTEXT_FOR_UMOUNT or FS_CONTEXT_FOR_EMERGENCY_RO will never access fc->fs_private? > + break; > > - fc->fs_private = kzalloc(sizeof(struct legacy_fs_context), GFP_KERNEL); > - if (!fc->fs_private) > - return -ENOMEM; > + case FS_CONTEXT_FOR_UMOUNT: > + case FS_CONTEXT_FOR_EMERGENCY_RO: > + break; > + } So `fc' can either be zeroed-out, when it comes from fc = kzalloc(), or contain some garbage otherwise. Would it make sense to zero-out `fc' regardless of its origin? > down_write(&sb->s_umount); > - if (!sb_rdonly(sb)) > - /* Might want to call ->init_fs_context(). */ > - ret = reconfigure_super(&fc); > + if (!sb_rdonly(sb)) { > + int ret; > + > + if (fc.fs_type->init_fs_context) > + ret = fc.fs_type->init_fs_context(&fc, NULL); > + else > + ret = legacy_init_fs_context(&fc, NULL); > + > + if (ret == 0) { > + ret = reconfigure_super(&fc); > + fc.ops->free(&fc); ^^^^^^^ Is ops->free() always !NULL? -ss