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.

Sign up for GitHub

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

Jump to bottom

Add note on f3probe assumptions to readme #145

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Add note on f3probe assumptions to readme #145

wants to merge 9 commits into from

Conversation

jowagner
Copy link
Contributor

@jowagner jowagner commented Sep 1, 2020

Add paragraph to readme explaining f3probe assumption and ask for reports on drives with different behaviour

jowagner and others added 4 commits September 1, 2020 13:03
@jowagner
Add note on f3probe assumptions to readme
894efa6 
address issue AltraMayor#144
@jowagner
remove duplicated empty line
767e4a8
@jowagner
Correct overflow protection
67337d7 
`sizeof()` returns the number of bytes, not bits, and since this is as an unsigned type and the value typically less than 10 the assertion currently does not fail but also fails to detect problems when `MAX_N_BLOCK_ORDER` is too big as the comparison is with the the very big number resulting from an unsigned overflow, typically 2^32-2.
@AltraMayor
Merge pull request AltraMayor#146 from jowagner/patch-2
10cc50f 
Correct overflow protection
AltraMayor
AltraMayor requested changes Sep 1, 2020
View reviewed changes
Copy link
Owner

@AltraMayor AltraMayor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add note on f3probe assumptions to readme

README.rst Outdated
Comment on lines 51 to 55
f3probe assumes that higher bits of the drive block address are simply
not connected in hardware. If you come across a fake drive that behaves
differently, please open an issue reporting the details.


Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This language is going to confuse lots of users. Most users just want to know if they have a fake drive or not, and what to do next. Helping them to do that is the way we fight flash fraud. It is better to add a section for people interested in the algorithm of f3probe and discuss technical aspects there.

Squash the next patch into this one since the next patch is just fixing this patch. You can use git rebase -i master at your branch.

@jowagner
Copy link
Contributor Author

jowagner commented Sep 2, 2020

Here a first draft. I also included a few words on f3write/read as a section technical details without this would be odd.

I did git rebase -i master as suggested (after syncing my master with your master). Not sure whether it is normal/a problem that the other change(s) now appear under "file(s) changed".

@jowagner
Copy link
Contributor Author

jowagner commented Sep 2, 2020

A few more lines to explain what f3read checks and how errors are reported.

@jowagner
Copy link
Contributor Author

jowagner commented Sep 3, 2020 •
edited
Loading

I added verbose output in https://github.com/jowagner/f3/tree/verbose and I now doubt I understand correctly how f3probe operates. Each line between the "Writing" lines shows how many blocks are validated by printing a v for each call of is_block_good(). f3probe writes a lot more data to the flash disk than I expect from my understanding of the code so far. Furthermore, if the search for bad blocks is a binary search and the search area doubles in size in each step the number of blocks verified in each step should increase at least by 1 in each step. However, the number of blocks verified in each step appears to be constant. It is still possible that the description in this PR is accurate but I cannot tell at the moment. Pease review carefully.

$ fdisk -l /dev/sdc
Disk /dev/sdc: 29.3 GiB, 31457280000 bytes, 61440000 sectors
Disk model: Micro Line      
Units: sectors of 1 * 512 = 512 bytes
Sector size (logical/physical): 512 bytes / 512 bytes
I/O size (minimum/optimal): 512 bytes / 512 bytes
$ ./f3probe --destructive --time-ops /dev/sdc
F3 probe 7.2
Copyright (C) 2010 Digirati Internet LTDA.
This is free software; see the source for copying conditions.

WARNING: Probing normally takes from a few seconds to 15 minutes, but
         it can take longer. Please be patient.


Writing 2048 blocks from 61437952 to 61439999.

Writing 4096 blocks from 61433856 to 61437951.
vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
Writing 8192 blocks from 61425664 to 61433855.
vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
Writing 16384 blocks from 61409280 to 61425663.
vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
Writing 32768 blocks from 61376512 to 61409279.
vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
Writing 65536 blocks from 61310976 to 61376511.
vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
Writing 131072 blocks from 61179904 to 61310975.
^C

@AltraMayor
Copy link
Owner

In order to avoid duplicate patches in this pull request, you need to git rebase master at your branch. You should git push --force afterward.

On the question of what f3probe is doing at the beginning of the process, it's trying to find the size of the cache of the drive. See find_cache_size() for details.

@jowagner
Copy link
Contributor Author

jowagner commented Sep 8, 2020

Thanks for the pointers, also in the related issue. I hope to find time soon to investigate a bit more so I can finalise the text.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AltraMayor AltraMayor AltraMayor requested changes

Assignees
No one assigned
Labels
Documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jowagner @AltraMayor