From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 243C73090C2 for ; Thu, 30 Apr 2026 19:46:48 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=170.10.129.124 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1777578410; cv=none; b=Nh48TLvZ2iGdDDK4I7JkVC+vI3jbGzXjhauOmP+y7uExE+ORcvePmMLpHDlDIfNzYlXU174xvezsnYajBmTzSNmrl3zGuRxaW3qjHHgvf7a1O6X3VlNG+aT5YKqO0HtVmjj9KbVOVaouY6WbXHcJW6qdz+7qpv9dhNHnsq6Ywcg= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1777578410; c=relaxed/simple; bh=p8xbgS6C1fOgo+36tywwUjCs6aIKbreuZE/pEOqFa6s=; h=Message-ID:Subject:From:To:Cc:Date:In-Reply-To:References: Content-Type:MIME-Version; b=SfOkxZSxn7iiHNnoIy2XwzblQ9K7P+d28KmnV1VYL4tSoA85LiunL9NefSS5TrWJqZda8Qc2wKTATDIp4+aFfpNqV2XU5a8WrSPGIltnpX7Ba9AQDIj0yNDFEaTBgnPcLfBknW85DmXid6pZkhQ3Yd2znpEaOacMslt9WhuUcUA= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com; spf=pass smtp.mailfrom=redhat.com; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b=c5um4thm; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b=OHXwpflq; arc=none smtp.client-ip=170.10.129.124 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="c5um4thm"; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b="OHXwpflq" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1777578408; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=7RkuhnTZK3QeVLQ8p6kdpJnT5whHnDkMltm4Ec0FV3U=; b=c5um4thmFUJn5PgBSO7Bo8w8yYksyNkK+Cpd8xM1wqFeiW7iu0Tr1rWWnuJb3on25xmQkz ySzox4fPM7t57V4K1QKd8kEY3Xn9QR0ZJulnYU5DKvpMEPgce74VK0Z+JnnZIXoYQZLT4f jp4GQm4mVWzMGtnxkTLkMS1KqkuZanQ= Received: from mail-yw1-f198.google.com (mail-yw1-f198.google.com [209.85.128.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-201-U-Rz4G1mMIS1d91kTk5rhg-1; Thu, 30 Apr 2026 15:46:46 -0400 X-MC-Unique: U-Rz4G1mMIS1d91kTk5rhg-1 X-Mimecast-MFC-AGG-ID: U-Rz4G1mMIS1d91kTk5rhg_1777578406 Received: by mail-yw1-f198.google.com with SMTP id 00721157ae682-79a670a5fe9so33605467b3.1 for ; Thu, 30 Apr 2026 12:46:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=google; t=1777578406; x=1778183206; darn=vger.kernel.org; h=mime-version:user-agent:content-transfer-encoding:references :in-reply-to:date:cc:to:from:subject:message-id:from:to:cc:subject :date:message-id:reply-to; bh=7RkuhnTZK3QeVLQ8p6kdpJnT5whHnDkMltm4Ec0FV3U=; b=OHXwpflqF+Crn94xUqwDS1yiEyEoLAJL2KjRkeA2PpLewZjJH3wpdcouy0TSYZKzkl 90HihHD7ShL2VC+6dNw9Tmaw/c0fdPZuSnG6pUbPPpZOObi4j0Pj351gyEroNbA0LQok RiraF5lFNIfulLaCM4/TSHPq3ZPs1V9a1BC+e8LCrhkPLE0XhD3PfCr0aecMTOya8Bzj Q4G+dNc9ecmi4s7bA0iMUw0QuTT8xINgoHCRUQ5lUX55je7JLXU5D5LR6nXMgPGbFSze SfdZPnsCLSg0ITN1xnHAFUM1HReZgr7JMuFKPgOz3UkSPdDMTR7dhNCYymPWAXbkGeIf s4bQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1777578406; x=1778183206; h=mime-version:user-agent:content-transfer-encoding:references :in-reply-to:date:cc:to:from:subject:message-id:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=7RkuhnTZK3QeVLQ8p6kdpJnT5whHnDkMltm4Ec0FV3U=; b=TsdVM3PRl2W9RRuBQNQ6aBloSMo1/bhvHNqkvT5/pMio/+b9j9UHuqmFFPB4fEOdic IzMXSiFBHKqkW+umx5UHlCekp5kd4di3tC9yTN1gg8FxRi04j4UA7wGTnd/73xSe4Qgf RwLR2Dgk5U4vXXiK/vS2OEfrsbAM5GdaUt5l2sfUljs9z3J95Q+Sfe9WQ5fE8P0YWCC/ 1P1hdHrSUfYDKwCD1pjez9I5WyHUGraMNq8s1N7jUR828MFe6dstYdeyeeULnCGiclHE 0MZrcQB6WFUwN77no4K5l5BhbARlUp4W4IWxZ8kqkSaHabgRs30KQpTAOr+QFXahTA26 iZvw== X-Forwarded-Encrypted: i=1; AFNElJ/8Ymyr8LbaPBKqbQQnQNSjVWOn8x0Y3nAGHF6TYH/Q2cVIcL+ekWD/eBH9zwE4lK2+nPcEOcwCWmxyNEM=@vger.kernel.org X-Gm-Message-State: AOJu0Yx2kLSTe2DACeTpvsOWiDdbSEY9NuYfAZKVrTqzCcef4qvA+l1Q 1h6f2SxcjdvsAeUNyR2a08pdopsX2LLjy12YNULG5SWBJcBxd/JT9No20cwhsIDZDF5N55ZRqn/ KIqvy5Lru/YVSKRSp/Kyh81D5f5m1u+tjEHwJjrgyDFn7i9nxEsYng4AOs5er1HXQAQ== X-Gm-Gg: AeBDiesSQW4oZCfynwxjKXrNT5Q184gLbL7HYUo9IroSqJQGMUGsPK8/YCneS2MI/df 0I8/bKijDUiqhFAe5gKD09mpS4cIA05+lBzVHd2m23qU2GfPbaU/EKW32DONMUf5mXmBZ1WmsBz x9MDVMQ1iO+kgveuxiN8J1gpjYcmyHL6fxrcmEpVLZHcROShhUmIFm9WpqCgOAQ1p2yePahSUMx 85oVFjSPWX2t+y1OL6Epk6pSU/nbhN80eMKpbYBxsVLulITgeUpaVVQ1FOY/gGoXb+XwSMwefE6 zTDuXO1D3JG+0F4NwvQxTxj+lkluGgsKM2dp9bmjM4Q0PU1LWftybu8amAQVKawE1lJu75Wbwbb cnGBmozYYzkX9yfqw6UqGqFzjRh22vNXTHl7XJSCvSejzLmgeT7uOxxxsqrXXuQA= X-Received: by 2002:a05:690c:6b01:b0:7b3:5872:694d with SMTP id 00721157ae682-7bd54803d33mr43303237b3.22.1777578405664; Thu, 30 Apr 2026 12:46:45 -0700 (PDT) X-Received: by 2002:a05:690c:6b01:b0:7b3:5872:694d with SMTP id 00721157ae682-7bd54803d33mr43302867b3.22.1777578405159; Thu, 30 Apr 2026 12:46:45 -0700 (PDT) Received: from li-4c4c4544-0032-4210-804c-c3c04f423534.ibm.com ([2600:1700:6476:1430::29]) by smtp.gmail.com with ESMTPSA id 00721157ae682-7bd6689ac83sm1461657b3.49.2026.04.30.12.46.43 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 30 Apr 2026 12:46:44 -0700 (PDT) Message-ID: <780d7cbfe8184eb2a90651393e0b3d28eee99ae0.camel@redhat.com> Subject: Re: [PATCH v4] hfsplus: fix null-ptr-deref by creating hidden dir on remount rw From: Viacheslav Dubeyko To: Deepanshu Kartikey , slava@dubeyko.com, glaubitz@physik.fu-berlin.de, frank.li@vivo.com Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, syzbot+c0ba772a362e70937dfb@syzkaller.appspotmail.com Date: Thu, 30 Apr 2026 12:46:41 -0700 In-Reply-To: <20260430020358.111052-1-kartikey406@gmail.com> References: <20260430020358.111052-1-kartikey406@gmail.com> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.60.0 (3.60.0-1.fc44app2) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 On Thu, 2026-04-30 at 07:33 +0530, Deepanshu Kartikey wrote: > hfsplus_reconfigure() does not create the hidden directory when > remounting from read-only to read-write, leaving sbi->hidden_dir > as NULL. This causes a null-ptr-deref when any subsequent > link/unlink/rename operation dereferences it. >=20 > Extract hidden directory creation into a helper and call it from > hfsplus_reconfigure() when switching to read-write mode. >=20 > Reported-by: syzbot+c0ba772a362e70937dfb@syzkaller.appspotmail.com > Closes: https://syzkaller.appspot.com/bug?extid=3Dc0ba772a362e70937dfb > Signed-off-by: Deepanshu Kartikey > --- > Changes in v4: > - Correct fix: extract hidden dir creation into helper and call > from hfsplus_reconfigure() on remount rw, as suggested by > Vyacheslav Dubeyko. >=20 > Changes in v3: > - Correct fix location: guard sbi->hidden_dir in hfsplus_link() > and hfsplus_unlink() in dir.c. >=20 > Changes in v2: > - Fixed commit message: hfsplus_delete_cat() has multiple callers, > not just hfsplus_unlink() as incorrectly stated in v1. > --- > fs/hfsplus/super.c | 88 ++++++++++++++++++++++++++++------------------ > 1 file changed, 53 insertions(+), 35 deletions(-) >=20 > diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c > index 7229a8ae89f9..8b5c39ce4d48 100644 > --- a/fs/hfsplus/super.c > +++ b/fs/hfsplus/super.c > @@ -372,15 +372,59 @@ static int hfsplus_statfs(struct dentry *dentry, st= ruct kstatfs *buf) > return 0; > } > =20 > +static int hfsplus_create_hidden_dir(struct super_block *sb) > +{ > + struct hfsplus_sb_info *sbi =3D HFSPLUS_SB(sb); > + struct inode *root =3D d_inode(sb->s_root); > + struct qstr str; > + int err; > + > + str.len =3D sizeof(HFSP_HIDDENDIR_NAME) - 1; > + str.name =3D HFSP_HIDDENDIR_NAME; The hfsplus_fill_super() has this initialization. It looks like duplication= . Maybe, we need to add this into hfsplus_reconfigure() and get str as input argument? > + > + mutex_lock(&sbi->vh_mutex); > + sbi->hidden_dir =3D hfsplus_new_inode(sb, root, S_IFDIR); > + if (!sbi->hidden_dir) { > + mutex_unlock(&sbi->vh_mutex); > + return -ENOMEM; > + } > + > + err =3D hfsplus_create_cat(sbi->hidden_dir->i_ino, root, > + &str, sbi->hidden_dir); > + if (err) { > + mutex_unlock(&sbi->vh_mutex); > + goto out_put_hidden_dir; > + } > + > + err =3D hfsplus_init_security(sbi->hidden_dir, root, &str); > + if (err =3D=3D -EOPNOTSUPP) > + err =3D 0; > + else if (err) { > + hfsplus_delete_cat(sbi->hidden_dir->i_ino, root, &str); > + mutex_unlock(&sbi->vh_mutex); > + goto out_put_hidden_dir; > + } > + > + mutex_unlock(&sbi->vh_mutex); > + hfsplus_mark_inode_dirty(sbi->hidden_dir, > + HFSPLUS_I_CAT_DIRTY); > + return 0; > + > +out_put_hidden_dir: > + iput(sbi->hidden_dir); > + sbi->hidden_dir =3D NULL; > + return err; > +} > + > static int hfsplus_reconfigure(struct fs_context *fc) > { > struct super_block *sb =3D fc->root->d_sb; > + struct hfsplus_sb_info *sbi =3D HFSPLUS_SB(sb); > =20 > sync_filesystem(sb); > if ((bool)(fc->sb_flags & SB_RDONLY) =3D=3D sb_rdonly(sb)) > return 0; > if (!(fc->sb_flags & SB_RDONLY)) { > - struct hfsplus_sb_info *sbi =3D HFSPLUS_SB(sb); > struct hfsplus_vh *vhdr =3D sbi->s_vhdr; > =20 > if (!(vhdr->attributes & cpu_to_be32(HFSPLUS_VOL_UNMNT))) { > @@ -401,6 +445,12 @@ static int hfsplus_reconfigure(struct fs_context *fc= ) > fc->sb_flags |=3D SB_RDONLY; > } > } > + /* > + * Create hidden dir if remounting read-write and it does > + * not exist - required for link/unlink/rename operations. > + */ > + if (!sb_rdonly(sb) && !sbi->hidden_dir) > + return hfsplus_create_hidden_dir(sb); It looks like hfsplus_reconfigure() is always false on a successful remount= . The sb->s_flags is updated by the VFS after reconfigure returns 0, not inside i= t. The correct guard is !(fc->sb_flags & SB_RDONLY), which reflects the negoti= ated new state. > return 0; > } > =20 > @@ -595,38 +645,9 @@ static int hfsplus_fill_super(struct super_block *sb= , struct fs_context *fc) > hfsplus_sync_fs(sb, 1); > =20 > if (!sbi->hidden_dir) { > - mutex_lock(&sbi->vh_mutex); > - sbi->hidden_dir =3D hfsplus_new_inode(sb, root, S_IFDIR); > - if (!sbi->hidden_dir) { > - mutex_unlock(&sbi->vh_mutex); > - err =3D -ENOMEM; > + err =3D hfsplus_create_hidden_dir(sb); > + if (err) > goto out_put_root; > - } > - err =3D hfsplus_create_cat(sbi->hidden_dir->i_ino, root, > - &str, sbi->hidden_dir); > - if (err) { > - mutex_unlock(&sbi->vh_mutex); > - goto out_put_hidden_dir; > - } > - > - err =3D hfsplus_init_security(sbi->hidden_dir, > - root, &str); > - if (err =3D=3D -EOPNOTSUPP) > - err =3D 0; /* Operation is not supported. */ > - else if (err) { > - /* > - * Try to delete anyway without > - * error analysis. > - */ What was wrong with the comment? Why have we lost it? > - hfsplus_delete_cat(sbi->hidden_dir->i_ino, > - root, &str); > - mutex_unlock(&sbi->vh_mutex); > - goto out_put_hidden_dir; > - } > - > - mutex_unlock(&sbi->vh_mutex); > - hfsplus_mark_inode_dirty(sbi->hidden_dir, > - HFSPLUS_I_CAT_DIRTY); Are you sure that you work with the latest sources? Because, current state = of this code is: hfsplus_mark_inode_dirty(HFSPLUS_CAT_TREE_I(sb), HFSPLUS_I_CAT_DIRTY); = =20 hfsplus_mark_inode_dirty(sbi->hidden_dir, HFSPLUS_I_CAT_DIRTY); > } > } > =20 > @@ -634,9 +655,6 @@ static int hfsplus_fill_super(struct super_block *sb,= struct fs_context *fc) > sbi->nls =3D nls; > return 0; > =20 > -out_put_hidden_dir: > - cancel_delayed_work_sync(&sbi->sync_work); Why cancel_delayed_work_sync() has been removed? The hfsplus_new_inode() ca= lls hfsplus_mark_mdb_dirty() which queues sync_work. So, nothing cancels the qu= eued work in the case of any issue. Thanks, Slava. > - iput(sbi->hidden_dir); > out_put_root: > dput(sb->s_root); > sb->s_root =3D NULL;