On Thu, Jun 18 2020 at 12:50pm -0400,
Sami Tolvanen <[email protected]> wrote:
> On Thu, Jun 18, 2020 at 11:44:45AM -0400, Mike Snitzer wrote:
> > I do not accept that panicing the system because of verity failure is
> > reasonable.
> >
> > In fact, even rebooting (via DM_VERITY_MODE_RESTART) looks very wrong.
> >
> > The device should be put in a failed state and left for admin recovery.
>
> That's exactly how the restart mode works on some Android devices. The
> bootloader sees the verification error and puts the device in recovery
> mode. Using the restart mode on systems without firmware support won't
> make sense, obviously.
OK, so I need further justification from Samsung why they are asking for
this panic mode.
Thanks,
Mike
Hello Dear DM-Verity maintainers.
Thank you for your reply.
I agreed with you that "the device should be put in a failed state and
left for admin recovery"
As dear Sami told us, When Android device occurred panic, restarting and
to save the logs to bootloader also recovery log.
Of course Using the restart mode on systems without firmware support
won't make sense.
However, on Android devices, restart or panic mode makes sense.
In android, the behavior is different depend on the binary type.
here are 3 type like user / userdebug / eng (engineering).
When kernel panic occurs, it operates as follows
* kernel panic in user binary(low)-> restart mode
* kernel panic in eng binary(mid) -> upload mode
It's actually at the debug level.
All users are set to low, but change it build option or others.
but Most users do not know.
You might think, "Why do you need a panic instead of reboot?"
To start with, It's easy to analyze what the device has problem.
If we use restart mode, it's difficult to analyze because device is
rebooted without logging.(not remain log)
And If use panic mode, samsung takes snapshots(save log etc) when
occurred panic.(Maybe other company or Android are same).
So We look for a debugging log and the analyze kind of problem in device
as well as dm-verity.
In the development stage, most of them are use in eng mode.
when panic occurs, it goes to upload mode, so it is convenient to
analyze whether it is HW problem / SW problem.
In most cases it was a hardware issue. Since we are a manufacturer, the
HW problem is also important.
Also, users using Android devices can recognize that there is a problem
with my device through a reboot.
Users don't know the exact reason, but they think that rebooting is wrong.
As mentioned above, in user mode, panic operates in reboot mode.
The user sees that device is rebooting and thinks there is a problem.
They uploads QnA to Samsung members App or visit service center for repair.
Then, developers need to get the log the device used by users to check
what the problem is. So We are using panic to get the log.
What's more, reboot/panic may seem wrong, but from a security
perspective, I think it's really important when looking at dm-verity.
Of course, I think the maintainers already know it.
To the important partition or Android devices system, will be protected
using dm-verity.
We can make the partition(want to protect) into a read-only partition,
compare the digest, and check whether there are any problems.
If a malicious user or hacker can damage the system or important
partition may change something.
At this time, we can defend against further hacking by generating a
panic or restart.
This will make the security feel strong. So reboot mode and panic mode
will be required.
We have long explained why we need it.
Through this, Samsung needs a panic mode, so please read it carefully
and give feedback.
Thank you :D
Jeonghyeon Lee
On 19/06/2020 02:09, Mike Snitzer wrote:
> On Thu, Jun 18 2020 at 12:50pm -0400,
> Sami Tolvanen <[email protected]> wrote:
>
>> On Thu, Jun 18, 2020 at 11:44:45AM -0400, Mike Snitzer wrote:
>>> I do not accept that panicing the system because of verity failure is
>>> reasonable.
>>>
>>> In fact, even rebooting (via DM_VERITY_MODE_RESTART) looks very wrong.
>>>
>>> The device should be put in a failed state and left for admin recovery.
>> That's exactly how the restart mode works on some Android devices. The
>> bootloader sees the verification error and puts the device in recovery
>> mode. Using the restart mode on systems without firmware support won't
>> make sense, obviously.
> OK, so I need further justification from Samsung why they are asking for
> this panic mode.
>
> Thanks,
> Mike
>
>
>
On 18/06/2020 19:09, Mike Snitzer wrote:
> On Thu, Jun 18 2020 at 12:50pm -0400,
> Sami Tolvanen <[email protected]> wrote:
>
>> On Thu, Jun 18, 2020 at 11:44:45AM -0400, Mike Snitzer wrote:
>>> I do not accept that panicing the system because of verity failure is
>>> reasonable.
>>>
>>> In fact, even rebooting (via DM_VERITY_MODE_RESTART) looks very wrong.
>>>
>>> The device should be put in a failed state and left for admin recovery.
>>
>> That's exactly how the restart mode works on some Android devices. The
>> bootloader sees the verification error and puts the device in recovery
>> mode. Using the restart mode on systems without firmware support won't
>> make sense, obviously.
>
> OK, so I need further justification from Samsung why they are asking for
> this panic mode.
I think when we have reboot already, panic is not much better :-)
Just please note that dm-verity is used not only in Android world (with own tooling)
but in normal Linux distributions, and I need to modify userspace (veritysetup) to support
and recognize this flag.
Please *always* increase minor dm-verity target version when adding a new feature
- we can then provide some better hint if it is not supported.
Thanks,
Milan
Dear Milan Broz.
Thank for your reply.
I didn't understand well, could you explain it in more detail?
For what reason isn't panic better?
Is it because there is a place to use other device-mapper?
Or other things? I just wonder. I would like to hear various
explanations and information.
I just wanted user to use what they wanted through the options(flags).
Yes, If adding a new feature, modify user-space to support.
Oh, I'm sorry :(
If when i suggested new patch, i will send you a patch that increased
minor version.
Thank you for all your detailed information.
Thanks.
JeongHyeon Lee
On 22/06/2020 16:58, Milan Broz wrote:
> On 18/06/2020 19:09, Mike Snitzer wrote:
>> On Thu, Jun 18 2020 at 12:50pm -0400,
>> Sami Tolvanen <[email protected]> wrote:
>>
>>> On Thu, Jun 18, 2020 at 11:44:45AM -0400, Mike Snitzer wrote:
>>>> I do not accept that panicing the system because of verity failure is
>>>> reasonable.
>>>>
>>>> In fact, even rebooting (via DM_VERITY_MODE_RESTART) looks very wrong.
>>>>
>>>> The device should be put in a failed state and left for admin recovery.
>>> That's exactly how the restart mode works on some Android devices. The
>>> bootloader sees the verification error and puts the device in recovery
>>> mode. Using the restart mode on systems without firmware support won't
>>> make sense, obviously.
>> OK, so I need further justification from Samsung why they are asking for
>> this panic mode.
> I think when we have reboot already, panic is not much better :-)
>
> Just please note that dm-verity is used not only in Android world (with own tooling)
> but in normal Linux distributions, and I need to modify userspace (veritysetup) to support
> and recognize this flag.
>
> Please *always* increase minor dm-verity target version when adding a new feature
> - we can then provide some better hint if it is not supported.
>
> Thanks,
> Milan
>
>
On 23/06/2020 01:53, JeongHyeon Lee wrote:
>
> For what reason isn't panic better?
I did not say panic is better, I said that while we have restart already in mainline dm-verity code,
panic() is almost the same, so I see no problem in merging this patch.
Stopping system this way could create more damage if it is not configured properly,
but I think it is quite common to stop the system as fast as possible if data system integrity
is violated...
> If when i suggested new patch, i will send you a patch that increased
> minor version.
I think Mike can fold-in version increase, if the patch is accepted.
But please include these version changes with every new feature.
Actually I am tracking it here for dm-verity as part of veritysetup userspace documentation:
https://gitlab.com/cryptsetup/cryptsetup/-/wikis/DMVerity
Thanks,
Milan
> On 22/06/2020 16:58, Milan Broz wrote:
>> On 18/06/2020 19:09, Mike Snitzer wrote:
>>> On Thu, Jun 18 2020 at 12:50pm -0400,
>>> Sami Tolvanen <[email protected]> wrote:
>>>
>>>> On Thu, Jun 18, 2020 at 11:44:45AM -0400, Mike Snitzer wrote:
>>>>> I do not accept that panicing the system because of verity failure is
>>>>> reasonable.
>>>>>
>>>>> In fact, even rebooting (via DM_VERITY_MODE_RESTART) looks very wrong.
>>>>>
>>>>> The device should be put in a failed state and left for admin recovery.
>>>> That's exactly how the restart mode works on some Android devices. The
>>>> bootloader sees the verification error and puts the device in recovery
>>>> mode. Using the restart mode on systems without firmware support won't
>>>> make sense, obviously.
>>> OK, so I need further justification from Samsung why they are asking for
>>> this panic mode.
>> I think when we have reboot already, panic is not much better :-)
>>
>> Just please note that dm-verity is used not only in Android world (with own tooling)
>> but in normal Linux distributions, and I need to modify userspace (veritysetup) to support
>> and recognize this flag.
>>
>> Please *always* increase minor dm-verity target version when adding a new feature
>> - we can then provide some better hint if it is not supported.
>>
>> Thanks,
>> Milan
>>
>>
Dear Milan Broz.
Thank you for answer my query.
I asked you again because i was confused.
Yes, I also looked at the document and get a lot of information or
studies related to dm-verity.
https://gitlab.com/cryptsetup/cryptsetup/-/wikis/DMVerity
Thank you : D
JeongHyeon Lee
On 23/06/2020 16:28, Milan Broz wrote:
> On 23/06/2020 01:53, JeongHyeon Lee wrote:
>> For what reason isn't panic better?
> I did not say panic is better, I said that while we have restart already in mainline dm-verity code,
> panic() is almost the same, so I see no problem in merging this patch.
>
> Stopping system this way could create more damage if it is not configured properly,
> but I think it is quite common to stop the system as fast as possible if data system integrity
> is violated...
>
>> If when i suggested new patch, i will send you a patch that increased
>> minor version.
> I think Mike can fold-in version increase, if the patch is accepted.
>
> But please include these version changes with every new feature.
>
> Actually I am tracking it here for dm-verity as part of veritysetup userspace documentation:
> https://gitlab.com/cryptsetup/cryptsetup/-/wikis/DMVerity
>
> Thanks,
> Milan
>
>> On 22/06/2020 16:58, Milan Broz wrote:
>>> On 18/06/2020 19:09, Mike Snitzer wrote:
>>>> On Thu, Jun 18 2020 at 12:50pm -0400,
>>>> Sami Tolvanen <[email protected]> wrote:
>>>>
>>>>> On Thu, Jun 18, 2020 at 11:44:45AM -0400, Mike Snitzer wrote:
>>>>>> I do not accept that panicing the system because of verity failure is
>>>>>> reasonable.
>>>>>>
>>>>>> In fact, even rebooting (via DM_VERITY_MODE_RESTART) looks very wrong.
>>>>>>
>>>>>> The device should be put in a failed state and left for admin recovery.
>>>>> That's exactly how the restart mode works on some Android devices. The
>>>>> bootloader sees the verification error and puts the device in recovery
>>>>> mode. Using the restart mode on systems without firmware support won't
>>>>> make sense, obviously.
>>>> OK, so I need further justification from Samsung why they are asking for
>>>> this panic mode.
>>> I think when we have reboot already, panic is not much better :-)
>>>
>>> Just please note that dm-verity is used not only in Android world (with own tooling)
>>> but in normal Linux distributions, and I need to modify userspace (veritysetup) to support
>>> and recognize this flag.
>>>
>>> Please *always* increase minor dm-verity target version when adding a new feature
>>> - we can then provide some better hint if it is not supported.
>>>
>>> Thanks,
>>> Milan
>>>
>>>
>