mirror of
https://github.com/SCST-project/scst.git
synced 2026-10-22 14:12:33 +00:00
scst_vdisk: Bound and synchronize FORMAT UNIT progress reporting
Manual review found division-by-zero and truncation bugs in REQUEST SENSE during FORMAT UNIT. No saved analyzer diagnostic directly reports these failures. do_div truncates the shifted 64-bit total to 32 bits: small totals and totals such as 1ULL << 48 produce a zero divisor. Completion can also wrap the 16-bit progress field back to zero. Use bounded 64-bit scaling and saturate completion at 0xffff. Publish initial counters with format_active and read/update them under flags_lock, avoiding inconsistent or torn 64-bit counter snapshots. Include the progress descriptor in the returned sense length. Add twelve KUnit cases for both sense formats, unknown/small totals, truncated divisors, large counters and completion. Exercise the actual REQUEST SENSE handler and validate the returned descriptor length.
This commit is contained in:
@@ -140,6 +140,7 @@ int vdisk_test_init(struct kunit *test)
|
||||
f->dev.block_shift = 9;
|
||||
f->dev.dev_dif_fn = vdisk_test_no_dif;
|
||||
f->vdev.dev = &f->dev;
|
||||
spin_lock_init(&f->vdev.flags_lock);
|
||||
f->vdev.dev_active = true;
|
||||
f->vdev.file_size = 16384;
|
||||
vdisk_test_file_init(&f->data, f);
|
||||
|
||||
@@ -178,7 +178,75 @@ static void vdisk_cache_test(struct kunit *test)
|
||||
p->error == -ENOMEM ? SAM_STAT_BUSY : SAM_STAT_CHECK_CONDITION);
|
||||
}
|
||||
|
||||
struct vdisk_format_case {
|
||||
const char *name;
|
||||
u64 done;
|
||||
u64 total;
|
||||
u16 expected;
|
||||
};
|
||||
|
||||
static const struct vdisk_format_case vdisk_format_cases[] = {
|
||||
{ "unknown_total", 0, 0, 0 },
|
||||
{ "small_start", 0, 8, 0 },
|
||||
{ "small_half", 4, 8, 0x8000 },
|
||||
{ "small_third", 1, 3, 0x5555 },
|
||||
{ "small_complete", 8, 8, 0xffff },
|
||||
{ "over_total", 9, 8, 0xffff },
|
||||
{ "old_divisor_truncates_to_zero", 1ULL << 47, 1ULL << 48, 0x8000 },
|
||||
{ "old_divisor_truncates_to_one", (1ULL << 47) + (1ULL << 15),
|
||||
(1ULL << 48) + (1ULL << 16), 0x8000 },
|
||||
{ "large_half", 1ULL << 62, 1ULL << 63, 0x8000 },
|
||||
{ "largest_half", 1ULL << 63, ULLONG_MAX, 0x8000 },
|
||||
{ "largest_nearly_complete", ULLONG_MAX - 1, ULLONG_MAX, 0xffff },
|
||||
{ "large_complete", 1ULL << 48, 1ULL << 48, 0xffff },
|
||||
};
|
||||
|
||||
static void vdisk_format_desc(const struct vdisk_format_case *p, char *desc)
|
||||
{
|
||||
snprintf(desc, KUNIT_PARAM_DESC_SIZE, "%s", p->name);
|
||||
}
|
||||
|
||||
KUNIT_ARRAY_PARAM(vdisk_format, vdisk_format_cases, vdisk_format_desc);
|
||||
|
||||
static void vdisk_format_progress_test(struct kunit *test)
|
||||
{
|
||||
const struct vdisk_format_case *p = test->param_value;
|
||||
struct vdisk_test_fixture *f = test->priv;
|
||||
u8 *sense = f->input[0];
|
||||
unsigned int descriptor;
|
||||
|
||||
f->vdev.format_active = 1;
|
||||
f->vdev.format_progress_done = p->done;
|
||||
f->vdev.format_progress_to_do = p->total;
|
||||
f->cmd.sg_cnt = 1;
|
||||
f->cmd.data_direction = SCST_DATA_READ;
|
||||
f->cmd.bufflen = 512;
|
||||
f->cmd.cdb[0] = REQUEST_SENSE;
|
||||
|
||||
for (descriptor = 0; descriptor < 2; descriptor++) {
|
||||
f->dev.d_sense = descriptor;
|
||||
scst_set_resp_data_len(&f->cmd, 512);
|
||||
memset(sense, 0xa5, 512);
|
||||
KUNIT_ASSERT_EQ(test, vdisk_exec_request_sense(&f->params), CMD_SUCCEEDED);
|
||||
KUNIT_EXPECT_EQ(test, f->cmd.status, SAM_STAT_GOOD);
|
||||
KUNIT_ASSERT_EQ(test, f->cmd.resp_data_len, descriptor ? 16 : 18);
|
||||
KUNIT_EXPECT_EQ(test, sense[0], descriptor ? 0x72 : 0x70);
|
||||
KUNIT_EXPECT_EQ(test, sense[descriptor ? 1 : 2], NOT_READY);
|
||||
KUNIT_EXPECT_EQ(test, sense[descriptor ? 2 : 12], 0x04);
|
||||
KUNIT_EXPECT_EQ(test, sense[descriptor ? 3 : 13], 0x04);
|
||||
KUNIT_EXPECT_EQ(test, sense[descriptor ? 12 : 15], 0x80);
|
||||
KUNIT_EXPECT_EQ(test, get_unaligned_be16(&sense[descriptor ? 13 : 16]),
|
||||
p->expected);
|
||||
if (descriptor) {
|
||||
KUNIT_EXPECT_EQ(test, sense[7], 8);
|
||||
KUNIT_EXPECT_EQ(test, sense[8], 2);
|
||||
KUNIT_EXPECT_EQ(test, sense[9], 6);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
static struct kunit_case vdisk_protocol_cases[] = {
|
||||
KUNIT_CASE_PARAM(vdisk_format_progress_test, vdisk_format_gen_params),
|
||||
KUNIT_CASE_PARAM(vdisk_cdb_test, vdisk_cdb_gen_params),
|
||||
KUNIT_CASE_PARAM(vdisk_cache_test, vdisk_cache_gen_params),
|
||||
{}
|
||||
|
||||
@@ -26,6 +26,7 @@
|
||||
#include <linux/version.h>
|
||||
#endif
|
||||
#include <linux/kernel.h>
|
||||
#include <linux/math64.h>
|
||||
#include <linux/wait.h>
|
||||
#include <linux/aio.h>
|
||||
#include <linux/file.h>
|
||||
@@ -1683,7 +1684,9 @@ static int vdisk_format_dif(struct scst_cmd *cmd, uint64_t start_lba, uint64_t b
|
||||
done -= full_len - err;
|
||||
}
|
||||
|
||||
spin_lock(&virt_dev->flags_lock);
|
||||
virt_dev->format_progress_done = done;
|
||||
spin_unlock(&virt_dev->flags_lock);
|
||||
}
|
||||
|
||||
out_free_data_page:
|
||||
@@ -1928,11 +1931,10 @@ static enum compl_status_e vdisk_exec_format_unit(struct vdisk_cmd_params *p)
|
||||
}
|
||||
|
||||
spin_lock(&virt_dev->flags_lock);
|
||||
virt_dev->format_active = 1;
|
||||
spin_unlock(&virt_dev->flags_lock);
|
||||
|
||||
virt_dev->format_progress_done = 0;
|
||||
virt_dev->format_progress_to_do = virt_dev->nblocks << SCST_DIF_TAG_SHIFT;
|
||||
virt_dev->format_active = 1;
|
||||
spin_unlock(&virt_dev->flags_lock);
|
||||
|
||||
if (virt_dev->thin_provisioned) {
|
||||
int rc = vdisk_unmap_range(cmd, virt_dev, 0, virt_dev->nblocks);
|
||||
@@ -3986,30 +3988,48 @@ out:
|
||||
return CMD_SUCCEEDED;
|
||||
}
|
||||
|
||||
static enum compl_status_e vdisk_exec_request_sense(struct vdisk_cmd_params *p)
|
||||
static u16 vdisk_format_progress(u64 done, u64 total)
|
||||
{
|
||||
u64 progress;
|
||||
|
||||
if (!total)
|
||||
return 0;
|
||||
if (done >= total)
|
||||
return USHRT_MAX;
|
||||
|
||||
/* Scale the fraction without overflowing either operand. */
|
||||
if (total <= ULLONG_MAX >> 16)
|
||||
progress = div64_u64(done << 16, total);
|
||||
else
|
||||
progress = div64_u64(done, total >> 16);
|
||||
|
||||
/* The large-volume approximation can round up by one progress unit. */
|
||||
return min_t(u64, progress, USHRT_MAX);
|
||||
}
|
||||
|
||||
SCST_KUNIT_VISIBLE
|
||||
enum compl_status_e vdisk_exec_request_sense(struct vdisk_cmd_params *p)
|
||||
{
|
||||
struct scst_cmd *cmd = p->cmd;
|
||||
struct scst_device *dev = cmd->dev;
|
||||
struct scst_vdisk_dev *virt_dev = dev->dh_priv;
|
||||
u64 done, total;
|
||||
bool format_active;
|
||||
int32_t length, sl;
|
||||
uint8_t *address;
|
||||
uint8_t b[SCST_STANDARD_SENSE_LEN];
|
||||
|
||||
TRACE_ENTRY();
|
||||
|
||||
/*
|
||||
* No need to make it volatile, because at worst we will have a couple
|
||||
* of extra commands refused after formatting actually finished, which
|
||||
* is acceptable.
|
||||
*/
|
||||
if (virt_dev->format_active) {
|
||||
uint64_t d, div;
|
||||
uint16_t v;
|
||||
/* Keep the active flag and both 64-bit counters in one consistent snapshot. */
|
||||
spin_lock(&virt_dev->flags_lock);
|
||||
format_active = virt_dev->format_active;
|
||||
done = virt_dev->format_progress_done;
|
||||
total = virt_dev->format_progress_to_do;
|
||||
spin_unlock(&virt_dev->flags_lock);
|
||||
|
||||
div = virt_dev->format_progress_to_do >> 16;
|
||||
d = virt_dev->format_progress_done;
|
||||
do_div(d, div);
|
||||
v = d;
|
||||
if (format_active) {
|
||||
u16 v = vdisk_format_progress(done, total);
|
||||
|
||||
TRACE_DBG("Format progress %d", v);
|
||||
|
||||
@@ -4021,6 +4041,7 @@ static enum compl_status_e vdisk_exec_request_sense(struct vdisk_cmd_params *p)
|
||||
uint8_t *p = &b[7];
|
||||
int o = 8;
|
||||
*p += 8;
|
||||
sl += 8;
|
||||
b[o] = 2;
|
||||
b[o + 1] = 6;
|
||||
b[o + 4] = 0x80;
|
||||
|
||||
@@ -86,6 +86,7 @@ struct scst_vdisk_dev {
|
||||
struct bio_set vdisk_bioset_struct;
|
||||
#endif
|
||||
|
||||
/* Protected by flags_lock, together with format_active. */
|
||||
uint64_t format_progress_to_do, format_progress_done;
|
||||
|
||||
int virt_id;
|
||||
|
||||
@@ -16,6 +16,7 @@ int vdisk_fsync(loff_t loff, loff_t len, struct scst_device *dev, gfp_t gfp_flag
|
||||
struct scst_cmd *cmd, bool async);
|
||||
void vdisk_on_free_cmd_params(const struct vdisk_cmd_params *p);
|
||||
enum compl_status_e vdisk_exec_sai_16(struct vdisk_cmd_params *p);
|
||||
enum compl_status_e vdisk_exec_request_sense(struct vdisk_cmd_params *p);
|
||||
enum compl_status_e fileio_exec_write(struct vdisk_cmd_params *p);
|
||||
enum compl_status_e fileio_exec_read(struct vdisk_cmd_params *p);
|
||||
bool vdisk_parse_offset(struct vdisk_cmd_params *p, struct scst_cmd *cmd);
|
||||
|
||||
Reference in New Issue
Block a user