linux-kernel - RE: [PATCH v2] exfat: check disk status during buffer write

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <162ee01dadd9e$19cbaa40$4d62fec0$@samsung.com>
Date: Wed, 24 Jul 2024 16:50:06 +0900
From: "Sungjong Seo" <sj1557.seo@...sung.com>
To: "'dongliang cui'" <cuidongliang390@...il.com>
Cc: "'Dongliang Cui'" <dongliang.cui@...soc.com>, <linkinjeon@...nel.org>,
	<linux-fsdevel@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<niuzhiguo84@...il.com>, <hao_hao.wang@...soc.com>, <ke.wang@...soc.com>,
	"'Zhiguo	Niu'" <zhiguo.niu@...soc.com>, <sj1557.seo@...sung.com>
Subject: RE: [PATCH v2] exfat: check disk status during buffer write

> On Wed, Jul 24, 2024 at 3:03 PM Sungjong Seo <sj1557.seo@...sung.com>
> wrote:
> >
[snip]
> > >
> > > +static int exfat_block_device_ejected(struct super_block *sb)
> > > +{
> > > +     struct backing_dev_info *bdi = sb->s_bdi;
> > > +
> > > +     return bdi->dev == NULL;
> > > +}
> > Have you tested with this again?
> Yes, I tested it in this way. The user side can receive the -ENODEV error
> after the device is ejected.
> dongliang.cui@...vice:/data/tmp # dd if=/dev/zero of=test.img bs=1M
> count=10240
> dd: test.img: write error: No such device
> 1274+0 records in
> 1273+1 records out
> 1335635968 bytes (1.2 G) copied, 8.060 s, 158 M/s
Oops!, write() seems to return ENODEV that man page does not have.
In exfat_map_cluster, it was necessary to distinguish and return error
values, but now that explicitly differentiated error messages will be
printed. So, why not return EIO again? It seem appropriate to return EIO
instead of ENODEV from the read/write syscall.

> 
> >
> > > +
> > >  static int exfat_get_block(struct inode *inode, sector_t iblock,
> > >               struct buffer_head *bh_result, int create)
> > >  {
> > > @@ -290,6 +298,9 @@ static int exfat_get_block(struct inode *inode,
> > > sector_t iblock,
> > >       sector_t valid_blks;
> > >       loff_t pos;
> > >
> > > +     if (exfat_block_device_ejected(sb))
> > This looks better than the modified location in the last patch.
> > However, the caller of this function may not be interested in exfat
> > error handling, so here we should call exfat_fs_error_ratelimit()
> > with an appropriate error message.
> Thank you for the reminder. I will make the changes in the next version.
Sounds good!

> 
> >
> > > +             return -ENODEV;
> > > +
> > >       mutex_lock(&sbi->s_lock);
> > >       last_block = EXFAT_B_TO_BLK_ROUND_UP(i_size_read(inode), sb);
> > >       if (iblock >= last_block && !create)
> > > --
> > > 2.25.1
> >
> >