Fix vitastor-cli rm-data removing data outside the requested range or pool #93

koolkhel · 2026-10-25T16:14:19+03:00

koolkhel commented

2026-10-25 16:14:19 +03:00

Description

rm-data could remove data other than the user asked for, and report success:

Fix vitastor-cli rm-data ignoring offsets with size suffixes. --min-offset/--max-offset were read with uint64_value(), which gives 0 for 8M, and 0 means "from the start" / "to the end". On a 32 MiB image, rm-data --pool 1 --inode 1 --min-offset 8M --max-offset 16M printed "Done" and zeroed all 32 MiB. Reversed ranges, --max-offset -8M, --max-offset 0 and an overflowing value did the same. Offsets are now decimal or 0x hex with an optional K/M/G/T suffix, and an invalid value, an empty or reversed range or an explicit --max-offset 0 fail with EINVAL before anything is removed. Numeric values from internal callers (modify --resize, rm) work as before.
Accept pool names and hex inode numbers in vitastor-cli rm-data. --pool <name> --inode <inode with another pool's bits> removed data in the other pool; --pool 2 --inode 0x1 "removed" inode 0 and exited 0. Pool names are now resolved like ls does (ENOENT for an unknown one), and --inode must be decimal or 0x hex.

The second commit reuses the number parser and the EINVAL checks of the first.

Tested: tests/test_rm.sh, extended for both commits (the second with a second pool); each part fails without its fix and passes with it. test_rm_degraded still passes.

Contributor Name

Yury Luneff (MIND Software LLC, https://mindsw.io)

CLA

I accept Vitastor CLA agreement: https://git.yourcmc.ru/vitalif/vitastor/src/branch/master/CLA-en.md

### Description rm-data could remove data other than the user asked for, and report success: - **Fix vitastor-cli rm-data ignoring offsets with size suffixes.** `--min-offset`/`--max-offset` were read with `uint64_value()`, which gives 0 for `8M`, and 0 means "from the start" / "to the end". On a 32 MiB image, `rm-data --pool 1 --inode 1 --min-offset 8M --max-offset 16M` printed "Done" and zeroed all 32 MiB. Reversed ranges, `--max-offset -8M`, `--max-offset 0` and an overflowing value did the same. Offsets are now decimal or 0x hex with an optional K/M/G/T suffix, and an invalid value, an empty or reversed range or an explicit `--max-offset 0` fail with EINVAL before anything is removed. Numeric values from internal callers (`modify --resize`, `rm`) work as before. - **Accept pool names and hex inode numbers in vitastor-cli rm-data.** `--pool <name> --inode <inode with another pool's bits>` removed data in the other pool; `--pool 2 --inode 0x1` "removed" inode 0 and exited 0. Pool names are now resolved like `ls` does (ENOENT for an unknown one), and `--inode` must be decimal or 0x hex. The second commit reuses the number parser and the EINVAL checks of the first. Tested: tests/test_rm.sh, extended for both commits (the second with a second pool); each part fails without its fix and passes with it. test_rm_degraded still passes. ### Contributor Name Yury Luneff (MIND Software LLC, https://mindsw.io) ### CLA - [x] I accept Vitastor CLA agreement: https://git.yourcmc.ru/vitalif/vitastor/src/branch/master/CLA-en.md

koolkhel added 2 commits 2026-10-25 16:14:20 +03:00

Fix vitastor-cli rm-data ignoring offsets with size suffixes 66434e3220

rm-data read --min-offset and --max-offset with uint64_value(), which
returns 0 for "8M", "-8M" or "0x1000", and 0 means "from the start" and
"no upper limit". So a range given with a suffix removed the whole
inode instead of a part of it, and the command reported success.

Parse offsets as decimal or 0x hex with an optional K/M/G/T suffix,
reject signs and values that overflow 64 bits, and fail with EINVAL
before removing anything if a value is invalid, the range is empty or
reversed, or --max-offset is explicitly 0 (omit it for no limit).

Accept pool names and hex inode numbers in vitastor-cli rm-data 1742cdc081

rm-data read --pool and --inode with uint64_value(), so a pool name
became 0 and the pool was silently taken from the inode's pool bits,
i.e. "--pool <other pool> --inode <full inode number>" removed data in
a different pool than requested. A hex --inode became 0 and rm-data
"removed" inode 0, reporting success.

Resolve pool names like ls does (ENOENT for an unknown pool), parse
--inode as decimal or 0x hex, and fail with EINVAL on anything else
before removing data.

vitalif commented

2026-10-25 22:37:39 +03:00

А вот это мержить не хочу, rm-data это низкоуровневая/внутренняя команда и никто не заявлял, что она должна поддерживать всякий сложный синтаксис

Единственное что добавил - валидацию численных значений - если уж передали нечисловое, будет EINVAL

А вот это мержить не хочу, rm-data это низкоуровневая/внутренняя команда и никто не заявлял, что она должна поддерживать всякий сложный синтаксис Единственное что добавил - валидацию численных значений - если уж передали нечисловое, будет EINVAL

vitalif closed this pull request

2026-10-25 22:37:39 +03:00

Pull request closed

Please reopen this pull request to perform a merge.

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: vitalif/vitastor#93

Allow edits from maintainers