Ticket #3749 (closed defect: fixed)

Opened 6 months ago

Last modified 5 months ago

Segfault in VFSs not setting block size in stat upon overwriting existing file

Reported by: zaytsev Owned by: andrew_b
Priority: major Milestone: 4.8.19
Component: mc-vfs Version: master
Keywords: Cc:
Blocked By: Blocking:
Branch state: merged Votes for changeset: committed-master

Description

See #3406 for the details.

Reproducer:

1) Connect by SFTP
2) Copy local file to remote
3) Overwrite local file on remote
4) Observe segfault if unlucky

Change History

comment:1 Changed 6 months ago by zaytsev

Andrew, so my guess is correct, what do we do about the segfault?

Obviously my dirty patch can be cleaned up (move the header to lib, remove disgusting copy-paste from sftp vfs, etc.), but I'm afraid we have a much bigger problem than that :-/ Basically any VFS that doesn't properly initialize st_blksize / st_blocks is broken after the optimization...

Do you think we should try to fix all sites where stat is being set, or rather add a wrapper around the block size detection thing?

comment:2 Changed 6 months ago by andrew_b

  • Status changed from new to accepted

Branch: 3749_vfs_blksize
Initial changeset:d49d3b906f1079de9ea55dfee01922e870cc0936

The work is in progress.

Last edited 6 months ago by andrew_b (previous) (diff)

comment:3 follow-up: ↓ 4 Changed 6 months ago by zaytsev

Do you think you could also customize blksize for ftpfs and fish? They seem to use 8k buffers which matched the default 8k buffer in the copy operation. It feels like 512 bytes will be... slow?

comment:4 in reply to: ↑ 3 Changed 6 months ago by andrew_b

  • Component changed from mc-core to mc-vfs
  • Branch state changed from no branch to on review

Replying to zaytsev:

Do you think you could also customize blksize for ftpfs and fish?

Done. Please review.

comment:5 Changed 6 months ago by zaytsev

Travis doesn't like it:

Sources not indented

src/consaver/cons.saver.c
src/vfs/fish/fish.c
src/vfs/ftpfs/ftpfs.c

comment:6 Changed 6 months ago by zaytsev

Now Travis is happy, and I left a couple of comments on GitHub?.

comment:7 follow-up: ↓ 8 Changed 5 months ago by zaytsev

I like the last commit very much, is there any other work to do in this branch, or I can vote for it?

comment:8 in reply to: ↑ 7 Changed 5 months ago by andrew_b

Replying to zaytsev:

is there any other work to do in this branch,

No.

or I can vote for it?

Please.

comment:9 Changed 5 months ago by zaytsev

  • Votes for changeset set to zaytsev
  • Branch state changed from on review to approved

comment:10 Changed 5 months ago by andrew_b

  • Status changed from accepted to testing
  • Votes for changeset changed from zaytsev to committed-master
  • Resolution set to fixed
  • Branch state changed from approved to merged

Merged to master: [9d735e02f17f0c77dd4916193de353b5ed266377].

git log --pretty=oneline 9478740..9d735e0

comment:11 Changed 5 months ago by andrew_b

  • Status changed from testing to closed
Note: See TracTickets for help on using tickets.