Received-SPF: pass (google.com: domain of linux-kernel+bounces-210955-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
From: "Kasireddy, Vivek" <vivek.kasireddy@intel.com>
To: Oscar Salvador <osalvador@suse.de>, Andrew Morton
	<akpm@linux-foundation.org>
CC: syzbot <syzbot+569ed13f4054f271087b@syzkaller.appspotmail.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>, "muchun.song@linux.dev"
	<muchun.song@linux.dev>, "syzkaller-bugs@googlegroups.com"
	<syzkaller-bugs@googlegroups.com>
Subject: RE: [syzbot] [mm?] general protection fault in
 dequeue_hugetlb_folio_nodemask (2)
Thread-Topic: [syzbot] [mm?] general protection fault in
 dequeue_hugetlb_folio_nodemask (2)
Thread-Index: AQHau+sDpqX17lR0T02oz/+gUJzVurHC0iiAgAAEmoCAAAGNAIAAvfWAgAAGEsA=
Date: Wed, 12 Jun 2024 05:55:20 +0000
Message-ID: <IA0PR11MB71857839BD4488371CAC4E9BF8C02@IA0PR11MB7185.namprd11.prod.outlook.com>
References: <0000000000004f12bb061a9acf07@google.com>
 <20240611103005.ae4170608bd15d63adee2492@linux-foundation.org>
 <ZmiNeTL5cZYmCbNf@localhost.localdomain>
 <ZmiOxhiWU-CE2ilg@localhost.localdomain>
 <ZmkuH8R2XJZEvJwh@localhost.localdomain>
In-Reply-To: <ZmkuH8R2XJZEvJwh@localhost.localdomain>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?MqF4IOQfC+zh5LHfeAUEfIx5oC7a+UswGVU1cDBw/5VFRR3eKZUfDluz+T5V?=
 =?us-ascii?Q?1TRT2P6iquAAu4gD+G4taP7Kl4RJAdcUuMYr17O12yBays0PGONoXCMp4lHr?=
 =?us-ascii?Q?qC8p4hKD406wX3ZK2fOZnrNRyj4QzfcM55C72u65FhZrSdLhn+D5+oAiELO1?=
 =?us-ascii?Q?i4dZB1HlvdtuJORKNljnKmo0XW9DyzHyTn0LuSO5JKeK40gQd6qAFeE9zxBW?=
 =?us-ascii?Q?4cR8/+eJS052pLxsXpg3+o2/J4NJrxvSqb+P2lBPABt/jauyPTdtqIYqmUGi?=
 =?us-ascii?Q?5ELTIRBVRq8gKmX8goyBDgtiX/9hETAE48e52nA1oiHmA2Mxhs9pp6acZKcK?=
 =?us-ascii?Q?Jj+vuBHB7XWK5Nv9rOS5z71/7GSN8n3pqDJAXpJjSoC4SX5Cr2pElyH7Q+MD?=
 =?us-ascii?Q?12luaqx1OIxVL7od0962n0/afRZNzeFdPPo56GH4+3ymLy9C5xVzGGroKMwH?=
 =?us-ascii?Q?dLt2TRWN/VMu2GOt1guK/eD5bZ3NaQVWJREv0cuahbYDtv8qWMamBKJsNTe6?=
 =?us-ascii?Q?2P3KoVHoDAyWAjVPlBJ6Ys2cIPT+1bpQXGVwyxywbJ8dLfKKEgdX/AqU+gIf?=
 =?us-ascii?Q?UJentjJ6tf4bA87jmo54J1pdJjT2tkeL7sNjPs54V/AAx/MVs2iACml1oVay?=
 =?us-ascii?Q?Pf8LZIb9VSKp8zCwuzqLAK0z1NpZWqCmxTcV3w5Euh9LdTG8iMeSJ06NmYcN?=
 =?us-ascii?Q?ZorMQ97b5ihNl92Fu8yDd4IeUxE3waywL0BL6KdZwf1E0/8C+miWXFcSYWKJ?=
 =?us-ascii?Q?hH5B4j8iOkkE2Uj3ocV2haCMQLEDbjFssm1iOY12Lx8w5/5kcVqpaNJdbfF7?=
 =?us-ascii?Q?qi4YEYK52dQROeyhaOgBBYRBRyxJQm4DtcDFuVb1frSJ1zD/4nfy9yLJ7+UR?=
 =?us-ascii?Q?yYc12WqRhn33QKub1fh0OhIQV4Kq2rcnPds6GMBuhBnERIylYPR5URcB9tiR?=
 =?us-ascii?Q?5SZQ5bC4IPP7GPNV0sifP+yUKFZrVt5h2C7n9+lXgx4UQIzZhdCnpgrE4YmJ?=
 =?us-ascii?Q?07E6kYllA3FCWOnFdT/AqiZ7XjJV1ExHVAx15C4SfQoVXrot6kSr1eQMwepZ?=
 =?us-ascii?Q?6cR0qWPFHCKKp8DtoABm268mivrUgeoL+RG7s/NwTjZczS+BdhSjPV8RQSBX?=
 =?us-ascii?Q?gY/VMtHvSXXp67rsfa3y/9kbLVzGN9kKEO9gh5SPnDP7K2imrKWjIw9wyOT2?=
 =?us-ascii?Q?RMfdryjSM4sZ2JxNl6nYHLo1RAI3xEgrQbWLU2jUaowcyvUqF/9bXfM+OBpG?=
 =?us-ascii?Q?BIi9h+nha7U2dTgKNp4Jy/X2ocaFORLPeCWZmbQYmo0TuxF2MgD6IBqTE+cn?=
 =?us-ascii?Q?hNo0sj/2+BO+im5hBfF61PkpnGi4AAy5w7AZy45Yp0BJDMuMs5DNb0OayQqc?=
 =?us-ascii?Q?mF+NuWGUoaFeLQtDdxjuPf+TYAc2ajGQsnvXvKq8CbIA2md/ob6p/Vmb2hqp?=
 =?us-ascii?Q?+GlU9LNnpnPmxA05kZ/DR84Vk0Lzm9MfaqIQXUNLjVKANNN6rF17fFZB3MyE?=
 =?us-ascii?Q?Tc/OzrrI3PUDs9ZYSnODM9Bg/1xDncW0mwrjO1Fz4vAz6anBAMSOQTLlota7?=
 =?us-ascii?Q?T0iXNsbAlHb4+51rjSOeBYTy9WgpPQZmEHJmDWIP?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: IA0PR11MB7185.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 4cb22e25-7f36-4484-021a-08dc8aa43e53
X-MS-Exchange-CrossTenant-originalarrivaltime: 12 Jun 2024 05:55:20.4423
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: 8pCLLynrjfHiclE1R52j1kizbxnhL4JNw+e9DyEVkXcBj2gXs2Jk0hEB0ZR+kq25cprnqEQc5cZgyQakRQAkwil+eX3HHzqXZS/nO3QWsp8=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA3PR11MB7462
X-OriginatorOrg: intel.com

Hi Oscar,

>=20
> On Tue, Jun 11, 2024 at 07:52:06PM +0200, Oscar Salvador wrote:
> > On Tue, Jun 11, 2024 at 07:46:33PM +0200, Oscar Salvador wrote:
> > > On Tue, Jun 11, 2024 at 10:30:05AM -0700, Andrew Morton wrote:
> > > > On Tue, 11 Jun 2024 03:34:25 -0700 syzbot
> <syzbot+569ed13f4054f271087b@syzkaller.appspotmail.com> wrote:
> > > >
> > > > > Hello,
> > > > >
> > > > > syzbot found the following issue on:
> > > >
> > > > Thanks.
> > > >
> > > > > Call Trace:
> > > > >  <TASK>
> > > > >  alloc_hugetlb_folio_nodemask+0xae/0x3f0 mm/hugetlb.c:2603
> > > > >  memfd_alloc_folio+0x15e/0x390 mm/memfd.c:75
> > > > >  memfd_pin_folios+0x1066/0x1720 mm/gup.c:3864
> > > > >  udmabuf_create+0x658/0x11c0 drivers/dma-buf/udmabuf.c:353
> > > > >  udmabuf_ioctl_create drivers/dma-buf/udmabuf.c:420 [inline]
> > > > >  udmabuf_ioctl+0x304/0x4f0 drivers/dma-buf/udmabuf.c:451
> > > > >  vfs_ioctl fs/ioctl.c:51 [inline]
> > > > >  __do_sys_ioctl fs/ioctl.c:907 [inline]
> > > > >  __se_sys_ioctl+0xfc/0x170 fs/ioctl.c:893
> > > > >  do_syscall_x64 arch/x86/entry/common.c:52 [inline]
> > > > >  do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
> > > > >  entry_SYSCALL_64_after_hwframe+0x77/0x7f
> > > >
> > > > I think we can pretty confidently point at the series "mm/gup:
> > > > Introduce memfd_pin_folios() for pinning memfd folios".  I'll drop =
the
> > > > v14 series.
> > >
> > > jfyi: I am trying to reproduce this locally.
> >
> > Actually, should not memfd_alloc_folio() pass htlb_alloc_mask() instead
> > of GFP_USER to alloc_hugetlb_folio_nodemask? Or at least do
> > GFP_HIGHUSER.
>=20
> Ok, I spot the issue.
> memfd_alloc_folio() was calling alloc_hugetlb_folio_nodemask with
> preferred_nid being NUMA_NO_NODE, but that is bad as
> dequeue_hugetlb_folio_nodemask will do:
>=20
> zonelist =3D node_zonelist(nid, gfp_mask)
>=20
> which will try to get node_zonelists from nid, but since nid is -1, heh.
>=20
> The below patch fixes the issue for me, but I think that the right place
> to fix this up would be alloc_hugetlb_folio_nodemask(), so we can place
> the numa_node_id() if preferred_nid =3D NUMA_NO_NODE in there as a safety
> net.
> This way we catch this before exploding in case the user was not careful
> enough.
>=20
> I will cook up a patch shortly.
Thank you for fixing this issue!

>=20
> Another thing is why memfd_alloc_folio uses GFP_USER instead of
> GFP_HIGHUSER, but that maybe because I see that memfd_pin_folios() is
> used by some DMA driver which might not have access to HIGH_MEMORY.
Right, memfd_pin_folios() is used by udmabuf driver for DMA but the reason
why GFP_USER is chosen is because I was following this code in gup.c:
                struct migration_target_control mtc =3D {
                        .nid =3D NUMA_NO_NODE,
                        .gfp_mask =3D GFP_USER | __GFP_NOWARN,
                        .reason =3D MR_LONGTERM_PIN,
                };

                if (migrate_pages(movable_folio_list, alloc_migration_targe=
t,
                                  NULL, (unsigned long)&mtc, MIGRATE_SYNC,
                                  MR_LONGTERM_PIN, NULL)) {

where, alloc_migration_target() does the following to allocate a hugetlb fo=
lio:
        if (folio_test_hugetlb(src)) {
                struct hstate *h =3D folio_hstate(src);

                gfp_mask =3D htlb_modify_alloc_mask(h, gfp_mask);
                return alloc_hugetlb_folio_nodemask(h, nid,
                                                mtc->nmask, gfp_mask,
                                                htlb_allow_alloc_fallback(m=
tc->reason));

but I somehow missed the early check in alloc_migration_target() where it d=
oes:
        if (nid =3D=3D NUMA_NO_NODE)
                nid =3D folio_nid(src);

Thanks,
Vivek

>=20
> diff --git a/mm/memfd.c b/mm/memfd.c
> index 8035c6325e3c..2692f0298adc 100644
> --- a/mm/memfd.c
> +++ b/mm/memfd.c
> @@ -68,12 +68,13 @@ static void memfd_tag_pins(struct xa_state *xas)
>  struct folio *memfd_alloc_folio(struct file *memfd, pgoff_t idx)
>  {
>  #ifdef CONFIG_HUGETLB_PAGE
> +	int nid =3D numa_node_id();
>  	struct folio *folio;
>  	int err;
>=20
>  	if (is_file_hugepages(memfd)) {
>  		folio =3D alloc_hugetlb_folio_nodemask(hstate_file(memfd),
> -						     NUMA_NO_NODE,
> +						     nid,
>  						     NULL,
>  						     GFP_USER,
>  						     false);
>=20
> >
> >
> > --
> > Oscar Salvador
> > SUSE Labs
> >
>=20
> --
> Oscar Salvador
> SUSE Labs