Re: [PATCH] omap-crypto - fix kernel oops and output buffer update

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 23/03/18 15:50, Tero Kristo wrote:
Hi Francis,

Your email program is still messing up things, the patch was sent as a multipart message. Use git-send-email tool or something similar if possible.

Also, there are at least two checkpatch issues (run scripts/checkpatch.pl -strict <patch>), in addition to the UTF-8 formatted message content itself.

WARNING: email address 'Francis Le Bourse<francis.lebourse@xxxxxx>                                                   ---' might be better as 'Francis Le Bourse <francis.lebourse@xxxxxx>                                ---'
#144:
Signed-off-by: Francis Le Bourse<francis.lebourse@xxxxxx>                                          ---

CHECK: Alignment should match open parenthesis
#157: FILE: drivers/crypto/omap-aes.c:101:
+static void omap_aes_read_n(struct omap_aes_dev *dd, u32 offset,
+                    u32 *value, int count)

Of course I copy pasted wrong checkpatch content here, this was meant for the AES one. Anyway, this one has its own checkpatch issues that must be fixed.

-Tero


I have a couple of additional comments, but can't add them as the patch content is an attachment (like, I would not add the WARN_ON.) Overall, the issue you have found is a legitimate problem, and should be fixed.

-Tero

On 22/03/18 16:55, Francis Le Bourse wrote:
Hello,
In omap_crypto_cleanup(),:
         if (orig && (flags & OMAP_CRYPTO_COPY_MASK))
             scatterwalk_map_and_copy(buf, orig, offset, len, 1);
implies that scatterwalk_map_and_copy() is called if flag is set to OMAP_CRYPTO_SG_COPIED. If the output buffer crosses a page boundary, the second and subsequent pages are overwritten.
The test should be : if(orig && (flags & OMAP_CRYPTO_DATA_COPIED))

The variable buf is always assigned but not used if flags == OMAP_CRYPTO_SG_COPIED.
In:
         buf = sg_virt(sg);
sg_page(sg) can be NULL, leading to a kernel crash:

     [   37.624352] Unable to handle kernel NULL pointer dereference at virtual address 00000000
     [   37.632502] pgd = ec150000
     [   37.635238] [00000000] *pgd=00000000
     [   37.638842] Internal error: Oops: 5 [#1] SMP ARM

     Entering kdb (current=0xec173080, pid 1125) on processor 0 Oops: (null)
     due to oops @ 0xc02c04f4
     CPU: 0 PID: 1125 Comm: ping Tainted: G         C 4.14.13-ti-r25 #62
     Hardware name: Generic DRA74X (Flattened Device Tree)
     task: ec173080 task.stack: ec0a8000
     PC is at pa> <ge_address+0x18/0xf4
     LR is at omap_crypto_cleanup+0x44/0x110
     pc : [<c02c04f4>]    lr : [<c0b33b78>]    psr: 40080113
     sp : ec0a9cf8  ip : ec0a9d20  fp : ec0a9d1c
     r10: 00000000  r9 : ec1918c0  r8 : 00000100
     r7 : 0000000a  r6 : ec0c6780  r5 : 00000000  r4 : 00000002
     r3 : 00000100  r2 : 00000000  r1 : ec1918c0  r0 : 00000000
     Flags: nZcv  IRQs on  FIQs on  Mode SVC_32  ISA ARM Segment none
     Control: 10c5387d  Table: ac15006a  DAC: 00000051
     CPU: 0 PID: 1125 Comm: ping Tainted: G         C 4.14.13-ti-r25 #62
     Hardware name: Generic DRA74X (Flattened Device Tree)

     [<c0d87df8>] (__dabt_svc) from [<c02c04f4>] (page_address+0x18/0xf4)
     [<c02c04f4>] (page_address) from [<c0b33b78>] (omap_crypto_cleanup+0x44/0x110)      [<c0b33b78>] (omap_crypto_cleanup) from [<c0b371f4>] (omap_aes_done_task+0x1a0/0x440)      [<c0b371f4>] (omap_aes_done_task) from [<c0149388>] (tasklet_action+0x70/0x104)      [<c0149388>] (tasklet_action) from [<c0101704>] (__do_softirq+0x124/0x378)
     [<c0101704>] (__do_softirq) from [<c0148d68>] (irq_exit+0xe8/0x150)
     [<c0148d68>] (irq_exit) from [<c01acd5c>] (__handle_domain_irq+0x70/0xc4)      [<c01acd5c>] (__handle_domain_irq) from [<c01015a0>] (gic_handle_irq+0x4c/0x88)      [<c01015a0>] (gic_handle_irq) from [<c0d87e8c>] (__irq_svc+0x6c/0x90)

Here it is simpler to reorder the code to fix the issues.




--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki



[Index of Archives]     [Kernel]     [Gnu Classpath]     [Gnu Crypto]     [DM Crypt]     [Netfilter]     [Bugtraq]

  Powered by Linux