[PATCH 1/2] ARC: gdbserver breakage in Big-Endian configuration #1

[PATCH 1/2] ARC: gdbserver breakage in Big-Endian configuration #1

[Vineet Gupta]

Exception handling keeps additional state (whether exception was Trap
and if it was due to a breakpoint) in pt_regs->event, a bitfield member

        unsigned long orig_r8:16, event:16;

A bitfield esentially has an "offset" and a "length". What I wasn't
aware of was that, bitfields in a union loose the "offset" attribute
and all of them are laid out at offset 0.

This obviously means that both @event and @orig_r8 will be incorrectly
referenced to at same "0" offset by "C" generated code which is
certainly wrong, not because both members are accessed, but because asm
code updates it at different address.

In Little Endian config, @event is at offset 0 and @orig_r8 (not
actively used at all) clashing with it is OK. However in Big Endian
config,

	ST 0xNNNN_EEEE, [addr]

writes 0xEEEE to @event (offset 2 in memory) while "C" code references
it from 0.

Needless to say, this causes ptrace machinery to not detect the breakpoint
scenario (and incorrect stop_pc returned to gdbserver).

------>8---------------
Thi issue is already fixed in mainline 3.11 kernel as part of commit:
502a0c775c7f0a "ARC: pt_regs update #5"

However that patch has lot more changes than I would like backporting,
hence this seperate change.
------>8---------------

Reported-by: Noam Camus <noamc@ezchip.com>
Cc: <stable@vger.kernel.org> # [3.9 and 3.10 only]
Tested-by: Anton Kolesov <akolesov@synopsys.com>
Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
---
 arch/arc/include/asm/ptrace.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arc/include/asm/ptrace.h b/arch/arc/include/asm/ptrace.h
index 6179de7..2046a89 100644
--- a/arch/arc/include/asm/ptrace.h
+++ b/arch/arc/include/asm/ptrace.h
@@ -52,12 +52,14 @@ struct pt_regs {
 
 	/*to distinguish bet excp, syscall, irq */
 	union {
+		struct {
 #ifdef CONFIG_CPU_BIG_ENDIAN
 		/* so that assembly code is same for LE/BE */
 		unsigned long orig_r8:16, event:16;
 #else
 		unsigned long event:16, orig_r8:16;
 #endif
+		};
 		long orig_r8_word;
 	};
 };
-- 
1.8.1.2

[PATCH 2/2] ARC: gdbserver breakage in Big-Endian configuration #2

[Vineet Gupta]

This is related to how how different asm instructions write to a
bitfield member.

In BE config, @orig_r8 is at offset 0 while @event is at offset 2.

"ST 0xNNNN_EEEE, [addr]" correctly writes 0xEEEE to @event, however
a half word write, "STW 0xEEEE, [addr]" does not in Big Endian while
works perfectly file for LE config (since @event is at offset 0).

------>8---------------
Thi issue is already fixed in mainline 3.11 kernel as part of commit:
502a0c775c7f0a "ARC: pt_regs update #5"

However that patch has lot more changes than I would like backporting,
hence this seperate change.
------>8---------------

Reported-by: Noam Camus <noamc@ezchip.com>
Cc: <stable@vger.kernel.org> # [3.9 and 3.10 only]
Tested-by: Anton Kolesov <akolesov@synopsys.com>
Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
---
 arch/arc/kernel/entry.S | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arc/kernel/entry.S b/arch/arc/kernel/entry.S
index 7b5f3ec..724de08 100644
--- a/arch/arc/kernel/entry.S
+++ b/arch/arc/kernel/entry.S
@@ -498,7 +498,7 @@ tracesys_exit:
 trap_with_param:
 
 	; stop_pc info by gdb needs this info
-	stw orig_r8_IS_BRKPT, [sp, PT_orig_r8]
+	st  orig_r8_IS_BRKPT, [sp, PT_orig_r8]
 
 	mov r0, r12
 	lr  r1, [efa]
@@ -727,7 +727,7 @@ not_exception:
 	; things to what they were, before returning from L2 context
 	;----------------------------------------------------------------
 
-	ldw  r9, [sp, PT_orig_r8]      ; get orig_r8 to make sure it is
+	ld   r9, [sp, PT_orig_r8]      ; get orig_r8 to make sure it is
 	brne r9, orig_r8_IS_IRQ2, 149f ; infact a L2 ISR ret path
 
 	ld r9, [sp, PT_status32]       ; get statu32_l2 (saved in pt_regs)
-- 
1.8.1.2

Re: [PATCH 1/2] ARC: gdbserver breakage in Big-Endian configuration #1

[Vineet Gupta]

Hi Greg,

I'd posted these patches for stable backport. Do I need to do anything more to get them included.

https://patchwork.kernel.org/patch/2841153/  [1/2] ARC: gdbserver breakage in Big-Endian configuration #1<https://patchwork.kernel.org/patch/2841153/>]
https://patchwork.kernel.org/patch/2841158/  [2/2] ARC: gdbserver breakage in Big-Endian configuration #2<https://patchwork.kernel.org/patch/2841158/>

Thx,
-Vineet


On 08/08/2013 07:23 PM, Vineet Gupta wrote:

Exception handling keeps additional state (whether exception was Trap
and if it was due to a breakpoint) in pt_regs->event, a bitfield member

        unsigned long orig_r8:16, event:16;

A bitfield esentially has an "offset" and a "length". What I wasn't
aware of was that, bitfields in a union loose the "offset" attribute
and all of them are laid out at offset 0.

This obviously means that both @event and @orig_r8 will be incorrectly
referenced to at same "0" offset by "C" generated code which is
certainly wrong, not because both members are accessed, but because asm
code updates it at different address.

In Little Endian config, @event is at offset 0 and @orig_r8 (not
actively used at all) clashing with it is OK. However in Big Endian
config,

        ST 0xNNNN_EEEE, [addr]

writes 0xEEEE to @event (offset 2 in memory) while "C" code references
it from 0.

Needless to say, this causes ptrace machinery to not detect the breakpoint
scenario (and incorrect stop_pc returned to gdbserver).

------>8---------------
Thi issue is already fixed in mainline 3.11 kernel as part of commit:
502a0c775c7f0a "ARC: pt_regs update #5"

However that patch has lot more changes than I would like backporting,
hence this seperate change.
------>8---------------

Reported-by: Noam Camus <noamc@ezchip.com><mailto:noamc@ezchip.com>
Cc: <stable@vger.kernel.org><mailto:stable@vger.kernel.org> # [3.9 and 3.10 only]
Tested-by: Anton Kolesov <akolesov@synopsys.com><mailto:akolesov@synopsys.com>
Signed-off-by: Vineet Gupta <vgupta@synopsys.com><mailto:vgupta@synopsys.com>
---
 arch/arc/include/asm/ptrace.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arc/include/asm/ptrace.h b/arch/arc/include/asm/ptrace.h
index 6179de7..2046a89 100644
--- a/arch/arc/include/asm/ptrace.h
+++ b/arch/arc/include/asm/ptrace.h
@@ -52,12 +52,14 @@ struct pt_regs {

        /*to distinguish bet excp, syscall, irq */
        union {
+               struct {
 #ifdef CONFIG_CPU_BIG_ENDIAN
                /* so that assembly code is same for LE/BE */
                unsigned long orig_r8:16, event:16;
 #else
                unsigned long event:16, orig_r8:16;
 #endif
+               };
                long orig_r8_word;
        };
 };

Re: [PATCH 1/2] ARC: gdbserver breakage in Big-Endian configuration #1

[greg Kroah-Hartman]

On Mon, Aug 19, 2013 at 09:08:49AM +0000, Vineet Gupta wrote:
> Hi Greg,
> 
> I'd posted these patches for stable backport. Do I need to do anything more to get them included.
> 
> https://patchwork.kernel.org/patch/2841153/  [1/2] ARC: gdbserver breakage in Big-Endian configuration #1<https://patchwork.kernel.org/patch/2841153/>]
> https://patchwork.kernel.org/patch/2841158/  [2/2] ARC: gdbserver breakage in Big-Endian configuration #2<https://patchwork.kernel.org/patch/2841158/>

I ignored them as I thought you were submitting them for upstream
inclusion.  If they are already in Linus's tree, then take a look at the
file, Documentation/stable_kernel_rules.txt for how to send a patch for
inclusion into a stable release (hint, I need to know the git commit id
that the patch has in Linus's tree, which I didn't see anywhere here.)

thanks,

greg k-h

Re: [PATCH 1/2] ARC: gdbserver breakage in Big-Endian configuration #1

[Vineet Gupta]

Hi Greg,

On 08/19/2013 08:27 PM, greg Kroah-Hartman wrote:
> On Mon, Aug 19, 2013 at 09:08:49AM +0000, Vineet Gupta wrote:
>> Hi Greg,
>>
>> I'd posted these patches for stable backport. Do I need to do anything more to get them included.
>>
>> https://patchwork.kernel.org/patch/2841153/  [1/2] ARC: gdbserver breakage in Big-Endian configuration #1<https://patchwork.kernel.org/patch/2841153/>]
>> https://patchwork.kernel.org/patch/2841158/  [2/2] ARC: gdbserver breakage in Big-Endian configuration #2<https://patchwork.kernel.org/patch/2841158/>
> 
> I ignored them as I thought you were submitting them for upstream
> inclusion.  If they are already in Linus's tree, then take a look at the
> file, Documentation/stable_kernel_rules.txt for how to send a patch for
> inclusion into a stable release (hint, I need to know the git commit id
> that the patch has in Linus's tree, which I didn't see anywhere here.)


I'm sure I mentioned the commit-id in the patch. Hint, please look at annotation
within --->8--- blocks in changelogs.

The caveat however is we can't apply those exact commits as it is as that warrants
more changes to be pulled in. However I'm going by the last stable kernel rule "It
or an equivalent fix must already exist in Linus' tree (upstream)."

Anyhow, since both these patches are extracted from same commit, I'll respin a
single patch (with a small addition - again part of the same commit) and send it
over. OK !

Thx,
-Vineet

Re: [PATCH 1/2] ARC: gdbserver breakage in Big-Endian configuration #1

[greg Kroah-Hartman]

On Tue, Aug 20, 2013 at 10:50:40AM +0530, Vineet Gupta wrote:
> Hi Greg,
> 
> On 08/19/2013 08:27 PM, greg Kroah-Hartman wrote:
> > On Mon, Aug 19, 2013 at 09:08:49AM +0000, Vineet Gupta wrote:
> >> Hi Greg,
> >>
> >> I'd posted these patches for stable backport. Do I need to do anything more to get them included.
> >>
> >> https://patchwork.kernel.org/patch/2841153/  [1/2] ARC: gdbserver breakage in Big-Endian configuration #1<https://patchwork.kernel.org/patch/2841153/>]
> >> https://patchwork.kernel.org/patch/2841158/  [2/2] ARC: gdbserver breakage in Big-Endian configuration #2<https://patchwork.kernel.org/patch/2841158/>
> > 
> > I ignored them as I thought you were submitting them for upstream
> > inclusion.  If they are already in Linus's tree, then take a look at the
> > file, Documentation/stable_kernel_rules.txt for how to send a patch for
> > inclusion into a stable release (hint, I need to know the git commit id
> > that the patch has in Linus's tree, which I didn't see anywhere here.)
> 
> 
> I'm sure I mentioned the commit-id in the patch. Hint, please look at annotation
> within --->8--- blocks in changelogs.

You're right, I missed that, sorry.

> The caveat however is we can't apply those exact commits as it is as that warrants
> more changes to be pulled in. However I'm going by the last stable kernel rule "It
> or an equivalent fix must already exist in Linus' tree (upstream)."
> 
> Anyhow, since both these patches are extracted from same commit, I'll respin a
> single patch (with a small addition - again part of the same commit) and send it
> over. OK !

That would be great, thanks.

greg k-h

[PATCH 0/2] ARC fixes for 3.9/3.10 stable backport

[Vineet Gupta]

Hi,

Please apply the following 2 patches to 3.9/3.10 series.
They have been extracted out of mainline patches which can't be individually
backported due to other dependencies (please see respective changelogs for
references).

Thx,
-Vineet

Vineet Gupta (2):
  ARC: gdbserver breakage in Big-Endian configuration #1
  ARC: gdbserver breakage in Big-Endian configuration #2

 arch/arc/include/asm/ptrace.h  | 2 ++
 arch/arc/include/asm/syscall.h | 5 ++---
 arch/arc/kernel/entry.S        | 4 ++--
 3 files changed, 6 insertions(+), 5 deletions(-)

-- 
1.8.1.2

[PATCH 1/2] ARC: gdbserver breakage in Big-Endian configuration #1

[Vineet Gupta]

[Based on mainline commit 502a0c775c7f0a: "ARC: pt_regs update #5"]

gdbserver needs @stop_pc, served by ptrace, but fetched from pt_regs
differently, based on in_brkpt_traps(), which in turn relies on
additional machine state in pt_regs->event bitfield.

        unsigned long orig_r8:16, event:16;

For big endian config, this macro was returning false, despite being in
breakpoint Trap exception, causing wrong @stop_pc to be returned to gdb.

Issue #1: In BE, @event above is at offset 2 in word, while a STW insn
          at offset 0 was used to update it. Resort to using ST insn
	  which updates the half-word at right location.

Issue #2: The union involving bitfields causes all the members to be
	  laid out at offset 0. So with fix #1 above, ASM was now
	  updating at offset 2, "C" code was still referencing at
	  offset 0. Fixed by wrapping bitfield in a struct.

Reported-by: Noam Camus <noamc@ezchip.com>
Cc: <stable@vger.kernel.org> # [3.9 and 3.10 only]
Tested-by: Anton Kolesov <akolesov@synopsys.com>
Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
---
 arch/arc/include/asm/ptrace.h | 2 ++
 arch/arc/kernel/entry.S       | 4 ++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arc/include/asm/ptrace.h b/arch/arc/include/asm/ptrace.h
index 6179de7..2046a89 100644
--- a/arch/arc/include/asm/ptrace.h
+++ b/arch/arc/include/asm/ptrace.h
@@ -52,12 +52,14 @@ struct pt_regs {
 
 	/*to distinguish bet excp, syscall, irq */
 	union {
+		struct {
 #ifdef CONFIG_CPU_BIG_ENDIAN
 		/* so that assembly code is same for LE/BE */
 		unsigned long orig_r8:16, event:16;
 #else
 		unsigned long event:16, orig_r8:16;
 #endif
+		};
 		long orig_r8_word;
 	};
 };
diff --git a/arch/arc/kernel/entry.S b/arch/arc/kernel/entry.S
index 7b5f3ec..724de08 100644
--- a/arch/arc/kernel/entry.S
+++ b/arch/arc/kernel/entry.S
@@ -498,7 +498,7 @@ tracesys_exit:
 trap_with_param:
 
 	; stop_pc info by gdb needs this info
-	stw orig_r8_IS_BRKPT, [sp, PT_orig_r8]
+	st  orig_r8_IS_BRKPT, [sp, PT_orig_r8]
 
 	mov r0, r12
 	lr  r1, [efa]
@@ -727,7 +727,7 @@ not_exception:
 	; things to what they were, before returning from L2 context
 	;----------------------------------------------------------------
 
-	ldw  r9, [sp, PT_orig_r8]      ; get orig_r8 to make sure it is
+	ld   r9, [sp, PT_orig_r8]      ; get orig_r8 to make sure it is
 	brne r9, orig_r8_IS_IRQ2, 149f ; infact a L2 ISR ret path
 
 	ld r9, [sp, PT_status32]       ; get statu32_l2 (saved in pt_regs)
-- 
1.8.1.2

Patch "ARC: gdbserver breakage in Big-Endian configuration #1" has been added to the 3.10-stable tree

[gregkh]

This is a note to let you know that I've just added the patch titled

    ARC: gdbserver breakage in Big-Endian configuration #1

to the 3.10-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     arc-gdbserver-breakage-in-big-endian-configuration-1.patch
and it can be found in the queue-3.10 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.


>From Vineet.Gupta1@synopsys.com  Thu Aug 22 15:32:58 2013
From: Vineet Gupta <Vineet.Gupta1@synopsys.com>
Date: Tue, 20 Aug 2013 13:38:10 +0530
Subject: ARC: gdbserver breakage in Big-Endian configuration #1
To: <stable@vger.kernel.org>
Cc: <gregkh@linuxfoundation.org>, <noamc@ezchip.com>, <Anton.Kolesov@synopsys.com>, <linux-kernel@vger.kernel.org>, Vineet Gupta <Vineet.Gupta1@synopsys.com>
Message-ID: <1376986091-30241-2-git-send-email-vgupta@synopsys.com>

From: Vineet Gupta <Vineet.Gupta1@synopsys.com>

[Based on mainline commit 502a0c775c7f0a: "ARC: pt_regs update #5"]

gdbserver needs @stop_pc, served by ptrace, but fetched from pt_regs
differently, based on in_brkpt_traps(), which in turn relies on
additional machine state in pt_regs->event bitfield.

        unsigned long orig_r8:16, event:16;

For big endian config, this macro was returning false, despite being in
breakpoint Trap exception, causing wrong @stop_pc to be returned to gdb.

Issue #1: In BE, @event above is at offset 2 in word, while a STW insn
          at offset 0 was used to update it. Resort to using ST insn
	  which updates the half-word at right location.

Issue #2: The union involving bitfields causes all the members to be
	  laid out at offset 0. So with fix #1 above, ASM was now
	  updating at offset 2, "C" code was still referencing at
	  offset 0. Fixed by wrapping bitfield in a struct.

Reported-by: Noam Camus <noamc@ezchip.com>
Tested-by: Anton Kolesov <akolesov@synopsys.com>
Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 arch/arc/include/asm/ptrace.h |    2 ++
 arch/arc/kernel/entry.S       |    4 ++--
 2 files changed, 4 insertions(+), 2 deletions(-)

--- a/arch/arc/include/asm/ptrace.h
+++ b/arch/arc/include/asm/ptrace.h
@@ -52,12 +52,14 @@ struct pt_regs {
 
 	/*to distinguish bet excp, syscall, irq */
 	union {
+		struct {
 #ifdef CONFIG_CPU_BIG_ENDIAN
 		/* so that assembly code is same for LE/BE */
 		unsigned long orig_r8:16, event:16;
 #else
 		unsigned long event:16, orig_r8:16;
 #endif
+		};
 		long orig_r8_word;
 	};
 };
--- a/arch/arc/kernel/entry.S
+++ b/arch/arc/kernel/entry.S
@@ -498,7 +498,7 @@ tracesys_exit:
 trap_with_param:
 
 	; stop_pc info by gdb needs this info
-	stw orig_r8_IS_BRKPT, [sp, PT_orig_r8]
+	st  orig_r8_IS_BRKPT, [sp, PT_orig_r8]
 
 	mov r0, r12
 	lr  r1, [efa]
@@ -723,7 +723,7 @@ not_exception:
 	; things to what they were, before returning from L2 context
 	;----------------------------------------------------------------
 
-	ldw  r9, [sp, PT_orig_r8]      ; get orig_r8 to make sure it is
+	ld   r9, [sp, PT_orig_r8]      ; get orig_r8 to make sure it is
 	brne r9, orig_r8_IS_IRQ2, 149f ; infact a L2 ISR ret path
 
 	ld r9, [sp, PT_status32]       ; get statu32_l2 (saved in pt_regs)


Patches currently in stable-queue which might be from Vineet.Gupta1@synopsys.com are

queue-3.10/arc-gdbserver-breakage-in-big-endian-configuration-1.patch
queue-3.10/arc-gdbserver-breakage-in-big-endian-configuration-2.patch

[PATCH 2/2] ARC: gdbserver breakage in Big-Endian configuration #2

[Vineet Gupta]

[Based on mainline commit 352c1d95e3220d0: "ARC: stop using
pt_regs->orig_r8"]

Stop using orig_r8 as it could get clobbered by ST in trap_with_param,
and further it is semantically not needed either.

Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
---
 arch/arc/include/asm/syscall.h | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/arc/include/asm/syscall.h b/arch/arc/include/asm/syscall.h
index 33ab304..29de098 100644
--- a/arch/arc/include/asm/syscall.h
+++ b/arch/arc/include/asm/syscall.h
@@ -18,7 +18,7 @@ static inline long
 syscall_get_nr(struct task_struct *task, struct pt_regs *regs)
 {
 	if (user_mode(regs) && in_syscall(regs))
-		return regs->orig_r8;
+		return regs->r8;
 	else
 		return -1;
 }
@@ -26,8 +26,7 @@ syscall_get_nr(struct task_struct *task, struct pt_regs *regs)
 static inline void
 syscall_rollback(struct task_struct *task, struct pt_regs *regs)
 {
-	/* XXX: I can't fathom how pt_regs->r8 will be clobbered ? */
-	regs->r8 = regs->orig_r8;
+	regs->r0 = regs->orig_r0;
 }
 
 static inline long
-- 
1.8.1.2

Patch "ARC: gdbserver breakage in Big-Endian configuration #2" has been added to the 3.10-stable tree

[gregkh]

This is a note to let you know that I've just added the patch titled

    ARC: gdbserver breakage in Big-Endian configuration #2

to the 3.10-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     arc-gdbserver-breakage-in-big-endian-configuration-2.patch
and it can be found in the queue-3.10 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.


>From Vineet.Gupta1@synopsys.com  Thu Aug 22 15:33:34 2013
From: Vineet Gupta <Vineet.Gupta1@synopsys.com>
Date: Tue, 20 Aug 2013 13:38:11 +0530
Subject: ARC: gdbserver breakage in Big-Endian configuration #2
To: <stable@vger.kernel.org>
Cc: <gregkh@linuxfoundation.org>, <noamc@ezchip.com>, <Anton.Kolesov@synopsys.com>, <linux-kernel@vger.kernel.org>, Vineet Gupta <Vineet.Gupta1@synopsys.com>
Message-ID: <1376986091-30241-3-git-send-email-vgupta@synopsys.com>

From: Vineet Gupta <Vineet.Gupta1@synopsys.com>

[Based on mainline commit 352c1d95e3220d0: "ARC: stop using
pt_regs->orig_r8"]

Stop using orig_r8 as it could get clobbered by ST in trap_with_param,
and further it is semantically not needed either.

Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 arch/arc/include/asm/syscall.h |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

--- a/arch/arc/include/asm/syscall.h
+++ b/arch/arc/include/asm/syscall.h
@@ -18,7 +18,7 @@ static inline long
 syscall_get_nr(struct task_struct *task, struct pt_regs *regs)
 {
 	if (user_mode(regs) && in_syscall(regs))
-		return regs->orig_r8;
+		return regs->r8;
 	else
 		return -1;
 }
@@ -26,8 +26,7 @@ syscall_get_nr(struct task_struct *task,
 static inline void
 syscall_rollback(struct task_struct *task, struct pt_regs *regs)
 {
-	/* XXX: I can't fathom how pt_regs->r8 will be clobbered ? */
-	regs->r8 = regs->orig_r8;
+	regs->r0 = regs->orig_r0;
 }
 
 static inline long


Patches currently in stable-queue which might be from Vineet.Gupta1@synopsys.com are

queue-3.10/arc-gdbserver-breakage-in-big-endian-configuration-1.patch
queue-3.10/arc-gdbserver-breakage-in-big-endian-configuration-2.patch