Skip to content

Conversation

@holiman
Copy link
Contributor

@holiman holiman commented Oct 29, 2014

This branch contains various fixes, firstly a rewrite of LF operations; "lf hid fskdemod" and "lf io fskdemod". The changes makes the code a lot cleaner, and also makes the 'lf hid fskdemod' work better, with less (none?) false readings. However, the changes breaks 'lf io fskdemod'. Hopefully that's temporary, as soon as someone can test it and provide some feedback, it should be easy to correct the indices and fix it.

The second batch of changes comes from coverity. I used the cloud scan (https://scan.coverity.com/) which is free for open source projects, and fixed ~30 issues. Some of them were false positives, other typical findings are illegal memory writes or reads. A lot of small issues. Because there were so many, I haven't been able to test each one individually, but I am fairly confident I didn't break anything that wasn't already broken.

Unless anyone yells 'STOP', I'll merge this soonish. If you want access to the coverity scan, send me a message. I thought I'd switch the scan to use the main repo (I did it on my clone) and set it up with a bit of automation.

@thomasthill
Copy link

great thanks nice job

On Wed, Oct 29, 2014 at 2:28 PM, Martin Holst Swende <
notifications@github.com> wrote:

This branch contains various fixes, firstly a rewrite of LF operations;
"lf hid fskdemod" and "lf io fskdemod". The changes makes the code a lot
cleaner, and also makes the 'lf hid fskdemod' work better, with less
(none?) false readings. However, the changes breaks 'lf io fskdemod'.
Hopefully that's temporary, as soon as someone can test it and provide some
feedback, it should be easy to correct the indices and fix it.

The second batch of changes comes from coverity. I used the cloud scan (
https://scan.coverity.com/) which is free for open source projects, and
fixed ~30 issues. Some of them were false positives, other typical findings
are illegal memory writes or reads. A lot of small issues. Because there
were so many, I haven't been able to test each one individually, but I am
fairly confident I didn't break anything that wasn't already broken.

Unless anyone yells 'STOP', I'll merge this soonish. If you want access to
the coverity scan, send me a message. I thought I'd switch the scan to use
the main repo (I did it on my clone) and set it up with a bit of

automation.

You can merge this Pull Request by running

git pull https://github.com/holiman/proxmark3 master

Or view, comment on, or merge it at:

#23
Commit Summary

  • Major refactoring of lfops, removed a lot of duplicate code
  • Refactoring low frequency operations, now 'lf hid fskdemod' is more
    stable. Also did changes to handling ioprox tags, this is yet untested, so
    until it's been tested it should be kept off 'stable' branch
  • First try att merging with head
  • Fixed compilation issues, but functionality not tested
  • Some minor changes and some documentation
  • Some more docs, also made lf hid fskdemod a bit more stable. Should
    be no more false readings now
  • Merge pull request Unique Tag Support #1 from holiman/ioprox_fixes
  • Fixed several issues found using a coverity-scan
  • More coverity findings
  • Coverity-fixes in armsrc
  • Merge pull request Added Kantech ioProx Support #2 from holiman/coverity_fixes

File Changes

Patch Links:


Reply to this email directly or view it on GitHub
#23.

@ikarus23
Copy link
Member

Sorry, right now I have no time to test the changes.
But I wanted to say thank you! This looks like hard work. And it looks like work nobody wants to do but everybody knows it has to be done. Especially the code cleanup/repair with coverity.

@buggii
Copy link

buggii commented Oct 30, 2014

Any commands+variable parameters you need to test? I will probably have time next week.

-----Original Message-----
From: ikarus notifications@github.com
To: Proxmark/proxmark3 proxmark3@noreply.github.com
Sent: gio, 30 ott 2014 9:47
Subject: Re: [proxmark3] LF operations fixes, and coverity code scan fixes (#23)

Sorry, right now I have no time to test the changes.
But I wanted to say thank you! This looks like hard work. And it looks like work nobody wants to do but everybody knows it is necessary. Especially the code cleanup/repair with coverity.


Reply to this email directly or view it on GitHub:
#23 (comment)

@holiman
Copy link
Contributor Author

holiman commented Oct 30, 2014

Well, it would of course be good if you also verified that you get less false readings when doing 'lf hid fskdemod'. Otherwise, feel free to peek around what files have been changed, I've been all over the place ... But it's mostly very small things.

holiman added a commit that referenced this pull request Oct 30, 2014
LF operations fixes, and coverity code scan fixes
@holiman holiman merged commit 0ce5620 into Proxmark:master Oct 30, 2014
@buggii
Copy link

buggii commented Nov 3, 2014

Hi my friend,

asper here ! Are you leaving the pm3 scene to get into HackRF and HydraBus one ? I will enjoy you but please do not leave the pm3 community

__________ Information from ESET NOD32 Antivirus, version of virus signature database 10664 (20141103) __________

The message was checked by ESET NOD32 Antivirus.

http://www.eset.com

@holiman
Copy link
Contributor Author

holiman commented Nov 4, 2014

We'll see. I think pm3 has a place, but perhaps there are other platforms more suitable for the majority of the usecases.
My HackRF involvement has nothing to do with RFID, but I am checking out Hydrabus for HF and Rfidler for LF.

@buggii
Copy link

buggii commented Nov 4, 2014

I already got a rfidler too! I must understand if hydra can be suitable for sniffing/emulating hf before acquiring another "spare time consuming tool" ;)

-----Original Message-----
From: Martin Holst Swende notifications@github.com
To: Proxmark/proxmark3 proxmark3@noreply.github.com
Cc: buggii buggii@hotmail.com
Sent: mar, 04 nov 2014 9:00
Subject: Re: [proxmark3] LF operations fixes, and coverity code scan fixes (#23)

We'll see. I think pm3 has a place, but perhaps there are other platforms more suitable for the majority of the usecases.
My HackRF involvement has nothing to do with RFID, but I am checking out Hydrabus for HF and Rfidler for LF.


Reply to this email directly or view it on GitHub:
#23 (comment)

@merlokk
Copy link
Contributor

merlokk commented Nov 4, 2014

Unfortunately hydra cant emulate Mifare Classic( If they dont understand why they will understand why then they try to make the code. As I see only one way exists( FPGA (

@holiman
Copy link
Contributor Author

holiman commented Nov 5, 2014

This is not the best place for a discussion (a closed pull-request completely unrelated to what we're talking about =D)), but merlokk, I'm very interested if you could explain why that would not be possible. Either here or in the proxmark forum, so bvernoux can be part of the discussion. I have not looked enough at the specs to form an opinion.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants