From: Amjad Alsharafi <amjadsharafi10@gmail.com>
To: qemu-devel@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>,
"open list:vvfat" <qemu-block@nongnu.org>
Subject: Re: [PATCH 3/3] ffvat: Fix reading files with non-continuous clusters
Date: Fri, 29 Mar 2024 14:31:02 +0800 [thread overview]
Message-ID: <ZgZgJlQQKTQAv7Gt@amjad-pc> (raw)
In-Reply-To: <20240327201231.31046-4-amjadsharafi10@gmail.com>
I noticed the issue in the commit message 'ffvat' should be 'vvfat',
I'll fix it in the next version.
On Thu, Mar 28, 2024 at 04:11:27AM +0800, Amjad Alsharafi wrote:
> When reading with `read_cluster` we get the `mapping` with
> `find_mapping_for_cluster` and then we call `open_file` for this
> mapping.
> The issue appear when its the same file, but a second cluster that is
> not immediately after it, imagine clusters `500 -> 503`, this will give
> us 2 mappings one has the range `500..501` and another `503..504`, both
> point to the same file, but different offsets.
>
> When we don't open the file since the path is the same, we won't assign
> `s->current_mapping` and thus accessing way out of bound of the file.
>
> From our example above, after `open_file` (that didn't open anything) we
> will get the offset into the file with
> `s->cluster_size*(cluster_num-s->current_mapping->begin)`, which will
> give us `0x2000 * (504-500)`, which is out of bound for this mapping and
> will produce some issues.
>
> Signed-off-by: Amjad Alsharafi <amjadsharafi10@gmail.com>
> ---
> block/vvfat.c | 21 ++++++++++++++-------
> 1 file changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/block/vvfat.c b/block/vvfat.c
> index cb3ab81e29..87165abc26 100644
> --- a/block/vvfat.c
> +++ b/block/vvfat.c
> @@ -1360,15 +1360,22 @@ static int open_file(BDRVVVFATState* s,mapping_t* mapping)
> {
> if(!mapping)
> return -1;
> + int new_path = 1;
> if(!s->current_mapping ||
> - strcmp(s->current_mapping->path,mapping->path)) {
> - /* open file */
> - int fd = qemu_open_old(mapping->path,
> + s->current_mapping->first_mapping_index!=mapping->first_mapping_index ||
> + (new_path = strcmp(s->current_mapping->path,mapping->path))) {
> +
> + if (new_path) {
> + /* open file */
> + int fd = qemu_open_old(mapping->path,
> O_RDONLY | O_BINARY | O_LARGEFILE);
> - if(fd<0)
> - return -1;
> - vvfat_close_current_file(s);
> - s->current_fd = fd;
> + if(fd<0)
> + return -1;
> + vvfat_close_current_file(s);
> +
> + s->current_fd = fd;
> + }
> + assert(s->current_fd);
> s->current_mapping = mapping;
> }
> return 0;
> --
> 2.44.0
>
next prev parent reply other threads:[~2024-03-29 6:32 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-27 20:11 [PATCH 0/3] vvfat: Fix bugs in writing and reading files Amjad Alsharafi
2024-03-27 20:11 ` [PATCH 1/3] vvfat: Fix bug in writing to middle of file Amjad Alsharafi
2024-03-27 20:11 ` [PATCH 2/3] vvfat: Fix usage of `info.file.offset` Amjad Alsharafi
2024-03-27 20:11 ` [PATCH 3/3] ffvat: Fix reading files with non-continuous clusters Amjad Alsharafi
2024-03-29 6:31 ` Amjad Alsharafi [this message]
2024-04-13 9:51 ` [PATCH 0/3] vvfat: Fix bugs in writing and reading files -- PING Amjad Alsharafi
2024-04-30 13:21 ` Amjad Alsharafi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZgZgJlQQKTQAv7Gt@amjad-pc \
--to=amjadsharafi10@gmail.com \
--cc=hreitz@redhat.com \
--cc=kwolf@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.