Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ISSUE]: installing .bin/.cue CDs directly to drive fails copy data #83

Open
1 task done
endrift opened this issue Nov 12, 2024 · 1 comment
Open
1 task done
Labels

Comments

@endrift
Copy link

endrift commented Nov 12, 2024

Checks

  • I have checked existing issues for duplicates and found none

Describe the issue

When attempting to use install_cd or install_dvd directly onto a drive (in my case, a /dev node on FreeBSD) when the source file is a .bin/.cue pair, while a plain UDF .iso file works fine. It exits with an opaque error: 00000002 (2): No such file or directory.

I attempted to debug this and discovered it's an issue with the data copying routine in iin_copy_ex--after copying several sectors, it attempts to read 512 2048-byte sectors, with an input_start_sector of 7134 out of the .bin file, which gets handled by img_base_read. output_start_sector is 242782072 but I'm not sure that's relevant. This redirects the read to al_read with 512 * the sector size specified in .cue file, which is 2352 bytes, or 1204224 bytes. However, the aligned_t has a smaller buffer_size, of 1048576.

I don't have the exact parameters anymore as my lldb session has scrolled out of my backlog by now, but I remember it fell through in al_read into the memmove branch, though I think the operation in question wound up being trivial. I believe usable_data_len was the same as buffer_size, so it ends up reading 0 bytes into the end of the buffer, as it's already full. This causes the read of 1204224 bytes to only return 1048576 bytes.

This short read from al_read gets passed directly out of img_base_read without checking if there was more that could be read. This results in iin_copy_ex getting confused that the read ended early at only ~445 sectors, and bailing out with this error.

I toyed with changing the number of sectors to read, but that just made problems worse. I also tried toying with the cache_size parameter for al_alloc, which did fix the issue, but I am unsure what the actual calculation should be.

This patch should work to change the rounding on the calculation to a ceil-equivalent, always rounding up to an extra integer instead of ever rounding down, but I am not familiar enough with the code to know if this is the right fix or even the right analysis:

diff --git a/iin_img_base.c b/iin_img_base.c
index 1aa7d94..b9db359 100644
--- a/iin_img_base.c
+++ b/iin_img_base.c
@@ -179,7 +179,7 @@ img_base_read(iin_t *iin,
             else {                        /* open the new input */
                 osal_handle_t in;
                 u_int32_t cache_size =
-                    ((img_base->raw_sector_size * IIN_NUM_SECTORS + img_base->raw_sector_size - 1) /
+                    ((img_base->raw_sector_size * IIN_NUM_SECTORS + part->device_sector_size - 1) /
                      part->device_sector_size);
                 result = osal_open(part->input_path, &in, 1); /* with no cache */
                 if (result == OSAL_OK) {

Console model

N/A

@endrift endrift added the bug label Nov 12, 2024
@endrift
Copy link
Author

endrift commented Nov 12, 2024

Forgot to mention: I hit this on both 32c296c and the v47 tag.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant