by Paolo Bonzini

On 25/11/2015 18:56, Ben Hutchings wrote:
> On Wed, 2015-11-25 at 12:31 +0100, Paolo Bonzini wrote:
>>
>> On 24/11/2015 23:33, Ben Hutchings wrote:
>>> 3.2.74-rc1 review patch. If anyone has any objections, please let me know.
>>>
>>> ------------------
>>>
>>> From: Paolo Bonzini <[email protected]>
>>>
>>> commit cbdb967af3d54993f5814f1cee0ed311a055377d upstream.
>>>
>>> This is needed to avoid the possibility that the guest triggers
>>> an infinite stream of #DB exceptions (CVE-2015-8104).
>>>
>>> VMX is not affected: because it does not save DR6 in the VMCS,
>>> it already intercepts #DB unconditionally.
>>>
>>> Reported-by: Jan Beulich <[email protected]>
>>> Signed-off-by: Paolo Bonzini <[email protected]>
>>> [bwh: Backported to 3.2: #DB and #BP did not share a function, and there is
>>> no operation pointer referring to it, so remove update_db_intercept()
>>> entirely]
>>
>> This is wrong, you still need to check the BP intercept in the
>> (incorrectly named as of 3.2) update_db_intercept function.
>>
>> Something like:
>>
>> -static void update_db_intercept(struct kvm_vcpu *vcpu)
>> +static void update_bp_intercept(struct kvm_vcpu *vcpu)
>> {
>> > > struct vcpu_svm *svm = to_svm(vcpu);
>>
>> -> > clr_exception_intercept(svm, DB_VECTOR);
>> > > clr_exception_intercept(svm, BP_VECTOR);
>> -
>> -> > if (svm->nmi_singlestep)
>> -> > > set_exception_intercept(svm, DB_VECTOR);
>> -
>> > > if (vcpu->guest_debug & KVM_GUESTDBG_ENABLE) {
>> -> > > if (vcpu->guest_debug &
>> -> > > (KVM_GUESTDBG_SINGLESTEP | KVM_GUESTDBG_USE_HW_BP))
>> -> > > > set_exception_intercept(svm, DB_VECTOR);
>> > > > if (vcpu->guest_debug & KVM_GUESTDBG_USE_SW_BP)
>> > > > > set_exception_intercept(svm, BP_VECTOR);
>> > > } else
>> > > vcpu->guest_debug = 0;
>> }
>>
>>
>> Then the calls in db_interception and enable_nmi_window can be removed,
>> but the one in svm_guest_debug is important.
>
> Sorry about that. I now have with this version:
>
> From: Paolo Bonzini <[email protected]>
> Date: Tue, 10 Nov 2015 09:14:39 +0100
> Subject: KVM: svm: unconditionally intercept #DB
>
> commit cbdb967af3d54993f5814f1cee0ed311a055377d upstream.
>
> This is needed to avoid the possibility that the guest triggers
> an infinite stream of #DB exceptions (CVE-2015-8104).
>
> VMX is not affected: because it does not save DR6 in the VMCS,
> it already intercepts #DB unconditionally.
>
> Reported-by: Jan Beulich <[email protected]>
> Signed-off-by: Paolo Bonzini <[email protected]>
> [bwh: Backported to 3.2, with thanks to Paolo:
> - update_db_bp_intercept() was called update_db_intercept()
> - The remaining call is in svm_guest_debug() rather than through svm_x86_ops]
> Signed-off-by: Ben Hutchings <[email protected]>
> ---
> arch/x86/kvm/svm.c | 14 +++-----------
> 1 file changed, 3 insertions(+), 11 deletions(-)
>
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -1015,6 +1015,7 @@ static void init_vmcb(struct vcpu_svm *s
> set_exception_intercept(svm, UD_VECTOR);
> set_exception_intercept(svm, MC_VECTOR);
> set_exception_intercept(svm, AC_VECTOR);
> + set_exception_intercept(svm, DB_VECTOR);
>
> set_intercept(svm, INTERCEPT_INTR);
> set_intercept(svm, INTERCEPT_NMI);
> @@ -1550,20 +1551,13 @@ static void svm_set_segment(struct kvm_v
> mark_dirty(svm->vmcb, VMCB_SEG);
> }
>
> -static void update_db_intercept(struct kvm_vcpu *vcpu)
> +static void update_bp_intercept(struct kvm_vcpu *vcpu)
> {
> struct vcpu_svm *svm = to_svm(vcpu);
>
> - clr_exception_intercept(svm, DB_VECTOR);
> clr_exception_intercept(svm, BP_VECTOR);
>
> - if (svm->nmi_singlestep)
> - set_exception_intercept(svm, DB_VECTOR);
> -
> if (vcpu->guest_debug & KVM_GUESTDBG_ENABLE) {
> - if (vcpu->guest_debug &
> - (KVM_GUESTDBG_SINGLESTEP | KVM_GUESTDBG_USE_HW_BP))
> - set_exception_intercept(svm, DB_VECTOR);
> if (vcpu->guest_debug & KVM_GUESTDBG_USE_SW_BP)
> set_exception_intercept(svm, BP_VECTOR);
> } else
> @@ -1581,7 +1575,7 @@ static void svm_guest_debug(struct kvm_v
>
> mark_dirty(svm->vmcb, VMCB_DR);
>
> - update_db_intercept(vcpu);
> + update_bp_intercept(vcpu);
> }
>
> static void new_asid(struct vcpu_svm *svm, struct svm_cpu_data *sd)
> @@ -1655,7 +1649,6 @@ static int db_interception(struct vcpu_s
> if (!(svm->vcpu.guest_debug & KVM_GUESTDBG_SINGLESTEP))
> svm->vmcb->save.rflags &=
> ~(X86_EFLAGS_TF | X86_EFLAGS_RF);
> - update_db_intercept(&svm->vcpu);
> }
>
> if (svm->vcpu.guest_debug &
> @@ -3557,7 +3550,6 @@ static void enable_nmi_window(struct kvm
> */
> svm->nmi_singlestep = true;
> svm->vmcb->save.rflags |= (X86_EFLAGS_TF | X86_EFLAGS_RF);
> - update_db_intercept(vcpu);
> }
>
> static int svm_set_tss_addr(struct kvm *kvm, unsigned int addr)
>

Thanks, this looks good.

Paolo

2015-11-25 23:05:16

by Luis Henriques

[permalink] [raw]

Subject: Re: [PATCH 3.2 22/52] ALSA: hda - Disable 64bit address for Creative HDA controllers

On Tue, Nov 24, 2015 at 10:33:59PM +0000, Ben Hutchings wrote:
> 3.2.74-rc1 review patch. If anyone has any objections, please let me know.
>
> ------------------
>
> From: Takashi Iwai <[email protected]>
>
> commit cadd16ea33a938d49aee99edd4758cc76048b399 upstream.
>
> We've had many reports that some Creative sound cards with CA0132
> don't work well. Some reported that it starts working after reloading
> the module, while some reported it starts working when a 32bit kernel
> is used. All these facts seem implying that the chip fails to
> communicate when the buffer is located in 64bit address.
>
> This patch addresses these issues by just adding AZX_DCAPS_NO_64BIT
> flag to the corresponding PCI entries. I casually had a chance to
> test an SB Recon3D board, and indeed this seems helping.
>
> Although this hasn't been tested on all Creative devices, it's safer
> to assume that this restriction applies to the rest of them, too. So
> the flag is applied to all Creative entries.
>
> Signed-off-by: Takashi Iwai <[email protected]>
> [bwh: Backported to 3.2: drop the change to AZX_DCAPS_PRESET_CTHDA]

Is there a reason for dropping this change? Adding the
AZX_DCAPS_NO_64BIT flag to the AZX_DCAPS_PRESET_CTHDA definition does
seem to make sense.

Cheers,
--
Lu?s

> Signed-off-by: Ben Hutchings <[email protected]>
> ---
> --- a/sound/pci/hda/hda_intel.c
> +++ b/sound/pci/hda/hda_intel.c
> @@ -3099,11 +3099,13 @@ static DEFINE_PCI_DEVICE_TABLE(azx_ids)
> .class = PCI_CLASS_MULTIMEDIA_HD_AUDIO << 8,
> .class_mask = 0xffffff,
> .driver_data = AZX_DRIVER_CTX | AZX_DCAPS_CTX_WORKAROUND |
> + AZX_DCAPS_NO_64BIT |
> AZX_DCAPS_RIRB_PRE_DELAY | AZX_DCAPS_POSFIX_LPIB },
> #else
> /* this entry seems still valid -- i.e. without emu20kx chip */
> { PCI_DEVICE(0x1102, 0x0009),
> .driver_data = AZX_DRIVER_CTX | AZX_DCAPS_CTX_WORKAROUND |
> + AZX_DCAPS_NO_64BIT |
> AZX_DCAPS_RIRB_PRE_DELAY | AZX_DCAPS_POSFIX_LPIB },
> #endif
> /* Vortex86MX */
>
> --
> To unsubscribe from this list: send the line "unsubscribe stable" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2015-11-25 23:11:06

by Luis Henriques

[permalink] [raw]

Subject: Re: [PATCH 3.2 38/52] Btrfs: fix race when listing an inode's xattrs

On Tue, Nov 24, 2015 at 10:33:59PM +0000, Ben Hutchings wrote:
> 3.2.74-rc1 review patch. If anyone has any objections, please let me know.
>
> ------------------
>
> From: Filipe Manana <[email protected]>
>
> commit f1cd1f0b7d1b5d4aaa5711e8f4e4898b0045cb6d upstream.
>
> When listing a inode's xattrs we have a time window where we race against
> a concurrent operation for adding a new hard link for our inode that makes
> us not return any xattr to user space. In order for this to happen, the
> first xattr of our inode needs to be at slot 0 of a leaf and the previous
> leaf must still have room for an inode ref (or extref) item, and this can
> happen because an inode's listxattrs callback does not lock the inode's
> i_mutex (nor does the VFS does it for us), but adding a hard link to an
> inode makes the VFS lock the inode's i_mutex before calling the inode's
> link callback.
>
> If we have the following leafs:
>
> Leaf X (has N items) Leaf Y
>
> [ ... (257 INODE_ITEM 0) (257 INODE_REF 256) ] [ (257 XATTR_ITEM 12345), ... ]
> slot N - 2 slot N - 1 slot 0
>
> The race illustrated by the following sequence diagram is possible:
>
> CPU 1 CPU 2
>
> btrfs_listxattr()
>
> searches for key (257 XATTR_ITEM 0)
>
> gets path with path->nodes[0] == leaf X
> and path->slots[0] == N
>
> because path->slots[0] is >=
> btrfs_header_nritems(leaf X), it calls
> btrfs_next_leaf()
>
> btrfs_next_leaf()
> releases the path
>
> adds key (257 INODE_REF 666)
> to the end of leaf X (slot N),
> and leaf X now has N + 1 items
>
> searches for the key (257 INODE_REF 256),
> with path->keep_locks == 1, because that
> is the last key it saw in leaf X before
> releasing the path
>
> ends up at leaf X again and it verifies
> that the key (257 INODE_REF 256) is no
> longer the last key in leaf X, so it
> returns with path->nodes[0] == leaf X
> and path->slots[0] == N, pointing to
> the new item with key (257 INODE_REF 666)
>
> btrfs_listxattr's loop iteration sees that
> the type of the key pointed by the path is
> different from the type BTRFS_XATTR_ITEM_KEY
> and so it breaks the loop and stops looking
> for more xattr items
> --> the application doesn't get any xattr
> listed for our inode
>
> So fix this by breaking the loop only if the key's type is greater than
> BTRFS_XATTR_ITEM_KEY and skip the current key if its type is smaller.
>
> Signed-off-by: Filipe Manana <[email protected]>
> [bwh: Backported to 3.2: s/found_key\.type/btrfs_key_type(\&found_key)/]

Actually, in my backport to 3.16 I decided to keep the usage of
'found_key.type' instead, as the usage of btrfs_key_type() has been
dropped with commit 962a298f3511 ("btrfs: kill the key type accessor
helpers").

Cheers,
--
Lu?s

> Signed-off-by: Ben Hutchings <[email protected]>
> ---
> fs/btrfs/xattr.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> --- a/fs/btrfs/xattr.c
> +++ b/fs/btrfs/xattr.c
> @@ -259,8 +259,10 @@ ssize_t btrfs_listxattr(struct dentry *d
> /* check to make sure this item is what we want */
> if (found_key.objectid != key.objectid)
> break;
> - if (btrfs_key_type(&found_key) != BTRFS_XATTR_ITEM_KEY)
> + if (btrfs_key_type(&found_key) > BTRFS_XATTR_ITEM_KEY)
> break;
> + if (btrfs_key_type(&found_key) < BTRFS_XATTR_ITEM_KEY)
> + goto next;
>
> di = btrfs_item_ptr(leaf, slot, struct btrfs_dir_item);
> if (verify_dir_item(root, leaf, di))
>
> --
> To unsubscribe from this list: send the line "unsubscribe stable" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2015-11-26 00:34:54

by Ben Hutchings

[permalink] [raw]

Subject: Re: [PATCH 3.2 22/52] ALSA: hda - Disable 64bit address for Creative HDA controllers

On Wed, 2015-11-25 at 23:05 +0000, Luis Henriques wrote:
> On Tue, Nov 24, 2015 at 10:33:59PM +0000, Ben Hutchings wrote:
> > 3.2.74-rc1 review patch.  If anyone has any objections, please let me know.
> >
> > ------------------
> >
> > From: Takashi Iwai <[email protected]>
> >
> > commit cadd16ea33a938d49aee99edd4758cc76048b399 upstream.
> >
> > We've had many reports that some Creative sound cards with CA0132
> > don't work well.  Some reported that it starts working after reloading
> > the module, while some reported it starts working when a 32bit kernel
> > is used.  All these facts seem implying that the chip fails to
> > communicate when the buffer is located in 64bit address.
> >
> > This patch addresses these issues by just adding AZX_DCAPS_NO_64BIT
> > flag to the corresponding PCI entries.  I casually had a chance to
> > test an SB Recon3D board, and indeed this seems helping.
> >
> > Although this hasn't been tested on all Creative devices, it's safer
> > to assume that this restriction applies to the rest of them, too.  So
> > the flag is applied to all Creative entries.
> >
> > Signed-off-by: Takashi Iwai <[email protected]>
> > [bwh: Backported to 3.2: drop the change to AZX_DCAPS_PRESET_CTHDA]
>
> Is there a reason for dropping this change?  Adding the
> AZX_DCAPS_NO_64BIT flag to the AZX_DCAPS_PRESET_CTHDA definition does
> seem to make sense.
[...]

The AZX_DCAPS_PRESET_CTHDA macro was introduced in 3.5.

Ben.

--
Ben Hutchings
Unix is many things to many people,
but it's never been everything to anybody.

Attachments:

signature.asc (811.00 B)
This is a digitally signed message part

2015-11-26 00:39:47

It looks good. Thank you. :-)

------- Original Message -------
Sender : Ben Hutchings<[email protected]>
Date : 2015-11-25 07:33 (GMT+09:00)
Title : [PATCH 3.2 16/52] ext4, jbd2: ensure entering into panic after recording an error in superblock

3.2.74-rc1 review patch. If anyone has any objections, please let me know.

------------------

From: Daeho Jeong

commit 4327ba52afd03fc4b5afa0ee1d774c9c5b0e85c5 upstream.

If a EXT4 filesystem utilizes JBD2 journaling and an error occurs, the
journaling will be aborted first and the error number will be recorded
into JBD2 superblock and, finally, the system will enter into the
panic state in "errors=panic" option. But, in the rare case, this
sequence is little twisted like the below figure and it will happen
that the system enters into panic state, which means the system reset
in mobile environment, before completion of recording an error in the
journal superblock. In this case, e2fsck cannot recognize that the
filesystem failure occurred in the previous run and the corruption
wouldn't be fixed.

Task A Task B
ext4_handle_error()
-> jbd2_journal_abort()
-> __journal_abort_soft()
-> __jbd2_journal_abort_hard()
| -> journal->j_flags |= JBD2_ABORT;
|
| __ext4_abort()
| -> jbd2_journal_abort()
| | -> __journal_abort_soft()
| | -> if (journal->j_flags & JBD2_ABORT)
| | return;
| -> panic()
|
-> jbd2_journal_update_sb_errno()

Tested-by: Hobin Woo
Signed-off-by: Daeho Jeong
Signed-off-by: Theodore Ts'o
Signed-off-by: Ben Hutchings
---
fs/ext4/super.c | 12 ++++++++++--
fs/jbd2/journal.c | 6 +++++-
include/linux/jbd2.h | 1 +
3 files changed, 16 insertions(+), 3 deletions(-)

--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -463,9 +463,13 @@ static void ext4_handle_error(struct sup
ext4_msg(sb, KERN_CRIT, "Remounting filesystem read-only");
sb->s_flags |= MS_RDONLY;
}
- if (test_opt(sb, ERRORS_PANIC))
+ if (test_opt(sb, ERRORS_PANIC)) {
+ if (EXT4_SB(sb)->s_journal &&
+ !(EXT4_SB(sb)->s_journal->j_flags & JBD2_REC_ERR))
+ return;
panic("EXT4-fs (device %s): panic forced after error\n",
sb->s_id);
+ }
}

void __ext4_error(struct super_block *sb, const char *function,
@@ -628,8 +632,12 @@ void __ext4_abort(struct super_block *sb
jbd2_journal_abort(EXT4_SB(sb)->s_journal, -EIO);
save_error_info(sb, function, line);
}
- if (test_opt(sb, ERRORS_PANIC))
+ if (test_opt(sb, ERRORS_PANIC)) {
+ if (EXT4_SB(sb)->s_journal &&
+ !(EXT4_SB(sb)->s_journal->j_flags & JBD2_REC_ERR))
+ return;
panic("EXT4-fs panic from previous error\n");
+ }
}

void ext4_msg(struct super_block *sb, const char *prefix, const char *fmt, ...)
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -1956,8 +1956,12 @@ static void __journal_abort_soft (journa

__jbd2_journal_abort_hard(journal);

- if (errno)
+ if (errno) {
jbd2_journal_update_sb_errno(journal);
+ write_lock(&journal->j_state_lock);
+ journal->j_flags |= JBD2_REC_ERR;
+ write_unlock(&journal->j_state_lock);
+ }
}

/**
--- a/include/linux/jbd2.h
+++ b/include/linux/jbd2.h
@@ -954,6 +954,7 @@ struct journal_s
#define JBD2_ABORT_ON_SYNCDATA_ERR 0x040 /* Abort the journal on file
* data write error in ordered
* mode */
+#define JBD2_REC_ERR 0x080 /* The errno in the sb has been recorded */

/*
* Function declarations for the journaling transaction and buffer????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m????????????I?