All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: Tejun Heo <tj@kernel.org>
Cc: "James E.J. Bottomley" <JBottomley@parallels.com>,
	Jens Axboe <axboe@kernel.dk>, lkml <linux-kernel@vger.kernel.org>,
	"linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>
Subject: Re: PING^7 (was Re: [PATCH v2 00/14] Corrections and customization of the SG_IO command whitelist (CVE-2012-4542))
Date: Fri, 24 May 2013 10:31:34 +0200	[thread overview]
Message-ID: <519F2566.2000008@redhat.com> (raw)
In-Reply-To: <CAOS58YODjkSF=V7wADqbn_z8c6bmj=r03hWxEs5B1O+1cd0n6g@mail.gmail.com>

Il 24/05/2013 10:02, Tejun Heo ha scritto:
> On Fri, May 24, 2013 at 4:13 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>> The same filtering table being applied to different classes of
>>> hardware is a software bug, but my point is that the practive
>>> essentially entrusts non-insignificant part of security enforcement to
>>> the hardware itself.  The variety of hardware in question is very wide
>>> and significant portion has historically been known to be flaky.
>>
>> Unproven theory, and contradicted by actual practice.  Bugs are more
>> common in the handling of borderline conditions, not in the handling of
>> unimplemented commands.
>>
>> If you want to be secure aginst buggy firmware, the commands you have to
>> block are READ and WRITE.  Check out the list of existing USB quirks.
> 
> Well, I'd actually much prefer disabling CDB whitelisting for all !MMC
> devices if at all possible. You're basically arguing that because what
> we have is already broken, it should be okay to break it further.
> Also, RW commands having more quirks doesn't necessarily indicate that
> they tend to be more broken. They just get hammered on a lot in
> various ways so problems on those commands tend to be more noticeable.

I agree intuition may not count, and it's perfectly possible that
firmware writers forgot a "break;" or put the wrong location in a jump
table, so that unimplemented commands give interesting results.

However, the _fact_ is that this might happen anyway with the buttload
of commands that are already enabled by the whitelist and that most
disks will never implement.

>> You need to allow more commands.
>> The count-me-out knob allows all commands.
>> You cannot always allow all commands.
>> Ergo, you cannot always use the count-me-out knob.
> 
> The thing is that both approaches aren't perfect here so you can make
> similar type of argument from the other side. If the system wants to
> give out raw hardware access to VMs, requiring it to delegate the
> device fully could be reasonable.

No, it is not unfortunately.  Allowing to do discards is one thing,
allowing to disrupt the settings of a SAN is another.  You can only
delegate the device fully in these cases:

(a) of course, if the guest is trusted;

(b) if QEMU is running as a confined user, then you can get by with a
userspace whitelist.  (Otherwise you're just a ptrace away from
arbitrary access, as you pointed out).

Unfortunately, there are _real_ problems that this patches fix, such as
log spews due to blocked commands, and these happen even if neither of
the above conditions is true.

Is there anything else I can do?  Sure, I can check for the presence of
the whitelist and hack the VPD pages to hide features that I know the
whitelist will block.  Is it the right thing to do?  In my opinion no.
It makes no sense to have raw device access _disable_ features compared
to emulation.

> Not ideal but widening direct
> command access by default is pretty nasty too.

I actually agree, and that's why I'm trying to balance the widening and
restricting.

Paolo

  reply	other threads:[~2013-05-24  8:31 UTC|newest]

Thread overview: 78+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-02-06 15:15 [PATCH v2 00/14] Corrections and customization of the SG_IO command whitelist (CVE-2012-4542) Paolo Bonzini
2013-02-06 15:15 ` [PATCH v2 01/14] sg_io: pass request_queue to blk_verify_command Paolo Bonzini
2013-02-06 15:15 ` [PATCH v2 02/14] sg_io: reorganize list of allowed commands Paolo Bonzini
2013-02-06 15:15 ` [PATCH v2 03/14] sg_io: use different default filters for each device class Paolo Bonzini
2013-02-06 15:15 ` [PATCH v2 04/14] sg_io: resolve conflicts between commands assigned to multiple classes (CVE-2012-4542) Paolo Bonzini
2013-02-06 15:15 ` [PATCH v2 05/14] sg_io: whitelist a few more commands for rare & obsolete device types Paolo Bonzini
2013-02-06 15:15 ` [PATCH v2 06/14] sg_io: whitelist another command for multimedia devices Paolo Bonzini
2013-02-06 15:15 ` [PATCH v2 07/14] sg_io: whitelist a few more commands for media changers Paolo Bonzini
2013-02-06 15:15 ` [PATCH v2 08/14] sg_io: whitelist a few more commands for tapes Paolo Bonzini
2013-02-06 15:15 ` [PATCH v2 09/14] sg_io: whitelist a few more commands for disks Paolo Bonzini
2013-02-06 15:15 ` [PATCH v2 10/14] sg_io: whitelist a few obsolete commands Paolo Bonzini
2013-02-06 15:15 ` [PATCH v2 11/14] sg_io: mark blk_set_cmd_filter_defaults as __init Paolo Bonzini
2013-02-06 15:15 ` [PATCH v2 12/14] sg_io: remove remnants of sysfs SG_IO filters Paolo Bonzini
2013-02-06 15:16 ` [PATCH v2 13/14] sg_io: introduce unpriv_sgio queue flag Paolo Bonzini
2013-02-06 15:16 ` [PATCH v2 14/14] sg_io: use unpriv_sgio to disable whitelisting for scanners Paolo Bonzini
2013-02-13  8:32 ` [PATCH v2 00/14] Corrections and customization of the SG_IO command whitelist (CVE-2012-4542) Paolo Bonzini
2013-02-13  8:32   ` Paolo Bonzini
2013-02-13 15:35   ` Douglas Gilbert
2013-02-13 15:48     ` Paolo Bonzini
2013-02-20 16:12 ` Paolo Bonzini
2013-02-20 16:12   ` Paolo Bonzini
2013-03-22 22:30   ` PING^2 " Paolo Bonzini
2013-04-04 18:18     ` PING^3 " Paolo Bonzini
2013-04-04 18:18       ` Paolo Bonzini
2013-04-17 12:26       ` PING^4 aka The Jon Corbet Effect " Paolo Bonzini
2013-04-27 13:31         ` PING^5 aka New ways to attract attentions " Paolo Bonzini
2013-04-27 13:31           ` Paolo Bonzini
2013-05-06 20:43   ` PING^6 " Paolo Bonzini
2013-05-22  6:35 ` PING^7 (was Re: [PATCH v2 00/14] Corrections and customization of the SG_IO command whitelist (CVE-2012-4542)) Paolo Bonzini
2013-05-22  9:32   ` Tejun Heo
2013-05-22  9:53     ` Paolo Bonzini
2013-05-22 10:02       ` Tejun Heo
2013-05-22 10:23         ` Paolo Bonzini
2013-05-22 12:07           ` James Bottomley
2013-05-22 14:07             ` Paolo Bonzini
2013-05-22 16:31               ` Paolo Bonzini
2013-05-22 16:31                 ` Paolo Bonzini
2013-05-22 13:41           ` Tejun Heo
2013-05-22 14:12             ` Paolo Bonzini
2013-05-22 14:30               ` Tejun Heo
2013-05-22 15:00                 ` Paolo Bonzini
2013-05-22 19:30                   ` Tejun Heo
2013-05-22 21:18                     ` Paolo Bonzini
2013-05-22 22:17                       ` Tejun Heo
2013-05-23  0:54                         ` Tejun Heo
2013-05-23  7:45                         ` Paolo Bonzini
2013-05-23  9:02                           ` Tejun Heo
2013-05-23  9:47                             ` Paolo Bonzini
2013-05-24  1:44                               ` Tejun Heo
2013-05-24  7:13                                 ` Paolo Bonzini
2013-05-24  8:02                                   ` Tejun Heo
2013-05-24  8:31                                     ` Paolo Bonzini [this message]
2013-05-24  9:07                                       ` Tejun Heo
2013-05-24  9:45                                         ` Paolo Bonzini
2013-05-24 22:20                                           ` Tejun Heo
2013-05-25  4:35                                     ` James Bottomley
2013-05-25  5:27                                       ` Christoph Hellwig
2013-05-25  7:05                                         ` Paolo Bonzini
2013-05-25  7:11                                           ` Christoph Hellwig
2013-05-25  7:21                                             ` Paolo Bonzini
2013-06-21 11:57                                           ` Christoph Hellwig
2013-05-25  8:37                                       ` Tejun Heo
2013-05-25 11:14                                         ` Paolo Bonzini
2013-05-25 12:48                                           ` Tejun Heo
2013-05-25 12:56                                             ` Paolo Bonzini
2013-05-22 15:03               ` Theodore Ts'o
2013-05-22 15:53                 ` Paolo Bonzini
2013-05-22 16:32                   ` Martin K. Petersen
2013-05-22 17:00                     ` Paolo Bonzini
2013-05-22 18:11                       ` Theodore Ts'o
2013-05-22 19:37                         ` Paolo Bonzini
2013-05-22 20:19                           ` Theodore Ts'o
2013-05-22 20:36                             ` Paolo Bonzini
2013-05-25  3:54                     ` Vladislav Bolkhovitin
2013-05-28 20:25                       ` Martin K. Petersen
2013-05-29  6:12                         ` Vladislav Bolkhovitin
2013-05-22 20:39                   ` Tejun Heo
2013-05-22 21:12                     ` Paolo Bonzini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=519F2566.2000008@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=JBottomley@parallels.com \
    --cc=axboe@kernel.dk \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=tj@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.