* [bug report] bcachefs: no checking for bch2_seek_pagecache_hole()
@ 2023-09-15 12:57 Dan Carpenter
2023-09-20 2:31 ` Kent Overstreet
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2023-09-15 12:57 UTC (permalink / raw)
To: kent.overstreet; +Cc: linux-bcachefs
Hello Kent Overstreet,
The patch e0750d947352: "bcachefs: Initial commit" from Mar 16, 2017
(linux-next), leads to the following Smatch static checker warning:
fs/bcachefs/fs-io.c:1013 bch2_seek_hole()
warn: error code type promoted to positive: 'next_hole'
fs/bcachefs/fs-io.c
977 static loff_t bch2_seek_hole(struct file *file, u64 offset)
978 {
979 struct bch_inode_info *inode = file_bch_inode(file);
980 struct bch_fs *c = inode->v.i_sb->s_fs_info;
981 struct btree_trans *trans;
982 struct btree_iter iter;
983 struct bkey_s_c k;
984 subvol_inum inum = inode_inum(inode);
985 u64 isize, next_hole = MAX_LFS_FILESIZE;
986 u32 snapshot;
987 int ret;
988
989 isize = i_size_read(&inode->v);
990 if (offset >= isize)
991 return -ENXIO;
992
993 trans = bch2_trans_get(c);
994 retry:
995 bch2_trans_begin(trans);
996
997 ret = bch2_subvolume_get_snapshot(trans, inum.subvol, &snapshot);
998 if (ret)
999 goto err;
1000
1001 for_each_btree_key_norestart(trans, iter, BTREE_ID_extents,
1002 SPOS(inode->v.i_ino, offset >> 9, snapshot),
1003 BTREE_ITER_SLOTS, k, ret) {
1004 if (k.k->p.inode != inode->v.i_ino) {
1005 next_hole = bch2_seek_pagecache_hole(&inode->v,
1006 offset, MAX_LFS_FILESIZE, 0, false);
1007 break;
1008 } else if (!bkey_extent_is_data(k.k)) {
1009 next_hole = bch2_seek_pagecache_hole(&inode->v,
1010 max(offset, bkey_start_offset(k.k) << 9),
1011 k.k->p.offset << 9, 0, false);
1012
bch2_seek_pagecache_hole returns llof_t. Negatives on error. This code
has no error checking?
--> 1013 if (next_hole < k.k->p.offset << 9)
1014 break;
1015 } else {
1016 offset = max(offset, bkey_start_offset(k.k) << 9);
1017 }
1018 }
1019 bch2_trans_iter_exit(trans, &iter);
1020 err:
1021 if (bch2_err_matches(ret, BCH_ERR_transaction_restart))
1022 goto retry;
1023
1024 bch2_trans_put(trans);
1025 if (ret)
1026 return ret;
1027
1028 if (next_hole > isize)
1029 next_hole = isize;
1030
1031 return vfs_setpos(file, next_hole, MAX_LFS_FILESIZE);
1032 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [bug report] bcachefs: no checking for bch2_seek_pagecache_hole()
2023-09-15 12:57 [bug report] bcachefs: no checking for bch2_seek_pagecache_hole() Dan Carpenter
@ 2023-09-20 2:31 ` Kent Overstreet
2023-09-26 14:53 ` Dan Carpenter
0 siblings, 1 reply; 3+ messages in thread
From: Kent Overstreet @ 2023-09-20 2:31 UTC (permalink / raw)
To: Dan Carpenter; +Cc: kent.overstreet, linux-bcachefs
On Fri, Sep 15, 2023 at 03:57:56PM +0300, Dan Carpenter wrote:
> Hello Kent Overstreet,
>
> The patch e0750d947352: "bcachefs: Initial commit" from Mar 16, 2017
> (linux-next), leads to the following Smatch static checker warning:
>
> fs/bcachefs/fs-io.c:1013 bch2_seek_hole()
> warn: error code type promoted to positive: 'next_hole'
>
> fs/bcachefs/fs-io.c
> 977 static loff_t bch2_seek_hole(struct file *file, u64 offset)
> 978 {
> 979 struct bch_inode_info *inode = file_bch_inode(file);
> 980 struct bch_fs *c = inode->v.i_sb->s_fs_info;
> 981 struct btree_trans *trans;
> 982 struct btree_iter iter;
> 983 struct bkey_s_c k;
> 984 subvol_inum inum = inode_inum(inode);
> 985 u64 isize, next_hole = MAX_LFS_FILESIZE;
> 986 u32 snapshot;
> 987 int ret;
> 988
> 989 isize = i_size_read(&inode->v);
> 990 if (offset >= isize)
> 991 return -ENXIO;
> 992
> 993 trans = bch2_trans_get(c);
> 994 retry:
> 995 bch2_trans_begin(trans);
> 996
> 997 ret = bch2_subvolume_get_snapshot(trans, inum.subvol, &snapshot);
> 998 if (ret)
> 999 goto err;
> 1000
> 1001 for_each_btree_key_norestart(trans, iter, BTREE_ID_extents,
> 1002 SPOS(inode->v.i_ino, offset >> 9, snapshot),
> 1003 BTREE_ITER_SLOTS, k, ret) {
> 1004 if (k.k->p.inode != inode->v.i_ino) {
> 1005 next_hole = bch2_seek_pagecache_hole(&inode->v,
> 1006 offset, MAX_LFS_FILESIZE, 0, false);
> 1007 break;
> 1008 } else if (!bkey_extent_is_data(k.k)) {
> 1009 next_hole = bch2_seek_pagecache_hole(&inode->v,
> 1010 max(offset, bkey_start_offset(k.k) << 9),
> 1011 k.k->p.offset << 9, 0, false);
> 1012
>
> bch2_seek_pagecache_hole returns llof_t. Negatives on error. This code
> has no error checking?
It doesn't return an error.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [bug report] bcachefs: no checking for bch2_seek_pagecache_hole()
2023-09-20 2:31 ` Kent Overstreet
@ 2023-09-26 14:53 ` Dan Carpenter
0 siblings, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2023-09-26 14:53 UTC (permalink / raw)
To: Kent Overstreet; +Cc: kent.overstreet, linux-bcachefs
On Tue, Sep 19, 2023 at 10:31:16PM -0400, Kent Overstreet wrote:
> On Fri, Sep 15, 2023 at 03:57:56PM +0300, Dan Carpenter wrote:
> > Hello Kent Overstreet,
> >
> > The patch e0750d947352: "bcachefs: Initial commit" from Mar 16, 2017
> > (linux-next), leads to the following Smatch static checker warning:
> >
> > fs/bcachefs/fs-io.c:1013 bch2_seek_hole()
> > warn: error code type promoted to positive: 'next_hole'
> >
> > fs/bcachefs/fs-io.c
> > 977 static loff_t bch2_seek_hole(struct file *file, u64 offset)
> > 978 {
> > 979 struct bch_inode_info *inode = file_bch_inode(file);
> > 980 struct bch_fs *c = inode->v.i_sb->s_fs_info;
> > 981 struct btree_trans *trans;
> > 982 struct btree_iter iter;
> > 983 struct bkey_s_c k;
> > 984 subvol_inum inum = inode_inum(inode);
> > 985 u64 isize, next_hole = MAX_LFS_FILESIZE;
> > 986 u32 snapshot;
> > 987 int ret;
> > 988
> > 989 isize = i_size_read(&inode->v);
> > 990 if (offset >= isize)
> > 991 return -ENXIO;
> > 992
> > 993 trans = bch2_trans_get(c);
> > 994 retry:
> > 995 bch2_trans_begin(trans);
> > 996
> > 997 ret = bch2_subvolume_get_snapshot(trans, inum.subvol, &snapshot);
> > 998 if (ret)
> > 999 goto err;
> > 1000
> > 1001 for_each_btree_key_norestart(trans, iter, BTREE_ID_extents,
> > 1002 SPOS(inode->v.i_ino, offset >> 9, snapshot),
> > 1003 BTREE_ITER_SLOTS, k, ret) {
> > 1004 if (k.k->p.inode != inode->v.i_ino) {
> > 1005 next_hole = bch2_seek_pagecache_hole(&inode->v,
> > 1006 offset, MAX_LFS_FILESIZE, 0, false);
> > 1007 break;
> > 1008 } else if (!bkey_extent_is_data(k.k)) {
> > 1009 next_hole = bch2_seek_pagecache_hole(&inode->v,
> > 1010 max(offset, bkey_start_offset(k.k) << 9),
> > 1011 k.k->p.offset << 9, 0, false);
> > 1012
> >
> > bch2_seek_pagecache_hole returns llof_t. Negatives on error. This code
> > has no error checking?
>
> It doesn't return an error.
It looks to the untrained eye that folio_hole_offset() can return
-ENOENT. And __filemap_get_folio() can definitely return -ENOMEM.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-09-26 14:53 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-15 12:57 [bug report] bcachefs: no checking for bch2_seek_pagecache_hole() Dan Carpenter
2023-09-20 2:31 ` Kent Overstreet
2023-09-26 14:53 ` Dan Carpenter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.