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

Boot: Do a better job of preserving certain parts of settings.txt #8673

Merged
merged 2 commits into from Mar 24, 2020

Conversation

@JosJuice
Copy link
Member

JosJuice commented Mar 16, 2020

This is a part of fixing https://bugs.dolphin-emu.org/issues/11930.

PR #8672 should be merged before this one, otherwise Wiimmfi users may run into a situation where a 7-day wait isn't necessary at first but becomes necessary later after playing a game from a different region, which I think would be rather confusing for them.

EDIT: By request from @Leseratte10 (see comments below), do not merge this PR before PR #8680.

Source/Core/Core/Boot/Boot_BS2Emu.cpp Outdated Show resolved Hide resolved
Source/Core/Core/Boot/Boot_BS2Emu.cpp Outdated Show resolved Hide resolved
@JosJuice JosJuice force-pushed the JosJuice:preserve-setting-txt branch from 126cf35 to 53bc508 Mar 16, 2020
@Leseratte10

This comment has been minimized.

Copy link
Contributor

Leseratte10 commented Mar 16, 2020

I just ran a script through the Wiimmfi database to grab all the existing serial number prefixes that ever connected to Wiimmfi, and it looks like the letters R and S can also exist as second char in the serial number. R is Argentina (NTSC-U). I don't know what country S is. From the logs it looks a lot like NTSC-U, but I'm not 100% sure.

@JosJuice JosJuice force-pushed the JosJuice:preserve-setting-txt branch from 53bc508 to c4dff25 Mar 16, 2020
@JosJuice

This comment has been minimized.

Copy link
Member Author

JosJuice commented Mar 16, 2020

I've added those two as NTSC-U.

JosJuice added 2 commits Mar 16, 2020
It doesn't necessarily start with RVL-001.
@JosJuice JosJuice force-pushed the JosJuice:preserve-setting-txt branch from c4dff25 to 3614e9f Mar 16, 2020
@JosJuice

This comment has been minimized.

Copy link
Member Author

JosJuice commented Mar 16, 2020

For curiosity's sake, I checked the setting.txt file of my Middle Eastern Wii, and it has the following:

AREA=USA
MODEL=RVL-001(UAE)
DVD=0
MPCH=0x7FFE
CODE=LMH
SERNO=[redacted]
VIDEO=NTSC
GAME=US

So the text in parentheses in MODEL isn't necessarily the same as AREA. Interesting...

@Leseratte10

This comment has been minimized.

Copy link
Contributor

Leseratte10 commented Mar 18, 2020

Yeah, it looks like "UAE" wouldn't be a valid value for AREA. Or the tool I'm using doesn't know that UAE would be valid. The "SettingEdit" I used to look at the settings.txt file only has JPN, USA, EUR, AUS, BRA, TWN, ROC, KOR, HKG, ASI, LTN and SAF in the dropdown for AREA. The "Model" value is a free text field. Maybe Nintendo decided on the UAE model region after they've already finalized the list of valid / possible AREAs?

Although it could just be that UAE is missing from the tool by accident, but then why would an UAE wii not have UAE as AREA ...

JosJuice added a commit to JosJuice/dolphin that referenced this pull request Mar 19, 2020
@Leseratte10

This comment has been minimized.

Copy link
Contributor

Leseratte10 commented Mar 20, 2020

I just tested this PR, but there is still a bug somewhere.

After importing the NAND backup, the settings are looking correct. Printing "decoded" to the console results in an output like this (assume my real serial number being LEH123456789):

AREA=EUR
MODEL=RVL-001(EUR)
DVD=0
MPCH=0x7FFE
CODE=LEH

SERNO=123456789
VIDEO=PAL
GAME=EU

Then, when I quit the game and start it again, it prints this instead:

AREA=EUR
MODEL=RVL-001(EUR)
DVD=0
MPCH=0x7FFE
CODE=LEH
SERNO=12345

Then, in any subsequent runs, it prints this:

AREA=EUR
MODEL=RVL-001(EUR)
DVD=0
MPCH=0x7FFE
CODE=LEH
SERNO=1234
VIDEO=
GAME=

So the last part of the serial number, and the VIDEO and GAME entries, are just gone.

Is it running into a size limit somewhere? Some part of Dolphin seems to corrupt the setting.txt again, and I haven't (yet?) been able to figure out where that happens. Do you have any idea?

@Leseratte10

This comment has been minimized.

Copy link
Contributor

Leseratte10 commented Mar 20, 2020

@JosJuice Okay, either I am reading this piece of code wrong (I don't think that's the case), or there is a fairly stupid bug in the SettingsHandler::Decrypt function.

Take a look at this code block that does the decryption:

const u8* str = m_buffer.data();
while (*str != 0)
{
  if (m_position >= m_buffer.size()) {
    return;  
  }
    
  decoded.push_back((u8)(m_buffer[m_position] ^ m_key));
  m_position++;
  str++;
  m_key = (m_key >> 31) | (m_key << 1);
}

Am I understanding this correctly that "str" is a pointer to the encoded / obfuscated data? This means that "str" is pointing to binary data, not to a string.
In turn, this means that when the encoded binary data contains a NULL byte, the code stops and thinks it's done.

When I replace the while (*str != 0) loop with a while (1) loop and only rely on the if condition to return at some point, everything is working just fine. A better solution would probably be to decrypt the char, and if it decrypts to a NULL byte, stop. But I don't know if Nintendo always ends their decrypted content with a NULL byte.

That would mean that currently, while decrypting a setting.txt, there is a 1/256 chance for each byte in the file, that the decryption just randomly stops because the encrypted value is a 0. That wouldn't just break the Wiimmfi console identification, but would break everything that relies on reading correct console data from the setting.txt and should (in my opinion) be fixed ASAP.

I'll open a new bug report for that because that doesn't really have anything to do with this PR. Please wait with merging this PR until this bug is fixed.

@Leseratte10

This comment has been minimized.

Copy link
Contributor

Leseratte10 commented Mar 20, 2020

I just opened issue 12019 for this decryption bug.

@Leseratte10

This comment has been minimized.

Copy link
Contributor

Leseratte10 commented Mar 24, 2020

Thanks for merging #8680 and thus fixing issue 12019, now this (#8673) should be the last PR that needs to be merged to make NAND import work properly, at least for all the serial numbers and other identifiers.

@leoetlino leoetlino merged commit 5b10f4b into dolphin-emu:master Mar 24, 2020
10 checks passed
10 checks passed
default Very basic checks passed, handed off to Buildbot.
Details
lint Build succeeded on builder lint
Details
pr-android Build succeeded on builder pr-android
Details
pr-deb-dbg-x64 Build succeeded on builder pr-deb-dbg-x64
Details
pr-deb-x64 Build succeeded on builder pr-deb-x64
Details
pr-freebsd-x64 Build succeeded on builder pr-freebsd-x64
Details
pr-osx-x64 Build succeeded on builder pr-osx-x64
Details
pr-ubu-x64 Build succeeded on builder pr-ubu-x64
Details
pr-win-dbg-x64 Build succeeded on builder pr-win-dbg-x64
Details
pr-win-x64 Build succeeded on builder pr-win-x64
Details
@JosJuice JosJuice deleted the JosJuice:preserve-setting-txt branch Mar 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.