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

[Xen-devel] [ovmf baseline-only test] 71235: all pass



This run is configured for baseline tests only.

flight 71235 ovmf real [real]
http://osstest.xs.citrite.net/~osstest/testlogs/logs/71235/

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf                 227fe49d5d4fe6513fc09766f1c9f3ff330ea845
baseline version:
 ovmf                 6bbd4a8f5f2a538e5017045ab75674ec106f7b54

Last test of basis    71234  2017-04-26 09:16:48 Z    0 days
Testing same since    71235  2017-04-26 18:19:26 Z    0 days    1 attempts

------------------------------------------------------------
People who touched revisions under test:
  Laszlo Ersek <lersek@xxxxxxxxxx>

jobs:
 build-amd64-xsm                                              pass    
 build-i386-xsm                                               pass    
 build-amd64                                                  pass    
 build-i386                                                   pass    
 build-amd64-libvirt                                          pass    
 build-i386-libvirt                                           pass    
 build-amd64-pvops                                            pass    
 build-i386-pvops                                             pass    
 test-amd64-amd64-xl-qemuu-ovmf-amd64                         pass    
 test-amd64-i386-xl-qemuu-ovmf-amd64                          pass    


------------------------------------------------------------
sg-report-flight on osstest.xs.citrite.net
logs: /home/osstest/logs
images: /home/osstest/images

Logs, config files, etc. are available at
    http://osstest.xs.citrite.net/~osstest/testlogs/logs

Test harness code can be found at
    http://xenbits.xensource.com/gitweb?p=osstest.git;a=summary


Push not applicable.

------------------------------------------------------------
commit 227fe49d5d4fe6513fc09766f1c9f3ff330ea845
Author: Laszlo Ersek <lersek@xxxxxxxxxx>
Date:   Tue Apr 25 14:44:36 2017 +0200

    ShellPkg/Shell: eliminate double-free in RunSplitCommand()
    
    Commit bd3fc8133b2b ("ShellPkg/App: Fix memory leak and save resources.",
    2016-05-20) added a FreePool() call for Split->SplitStdIn, near end of the
    RunSplitCommand(), right after the same shell file was closed with
    CloseFile(). The argument was:
    
    > 1) RunSplitCommand() allocates the initial SplitStdOut via
    >    CreateFileInterfaceMem(). Free SplitStdIn after the swap to fix
    >    the memory leak.
    
    There is no memory leak actually, and the FreePool() call in question
    constitutes a double-free:
    
    (a) This is how the handle is established:
    
        ConvertEfiFileProtocolToShellHandle (
          CreateFileInterfaceMem (Unicode),
          NULL
          );
    
        CreateFileInterfaceMem() allocates an EFI_FILE_PROTOCOL_MEM object and
        populates it fully. ConvertEfiFileProtocolToShellHandle() allocates
        some administrative structures and links the EFI_FILE_PROTOCOL_MEM
        object into "mFileHandleList".
    
    (b) EFI_SHELL_PROTOCOL.CloseFile() is required to close the
        SHELL_FILE_HANDLE and to release all associated data. Accordingly,
        near the end of RunSplitCommand(), we have:
    
        EfiShellClose()
          ShellFileHandleRemove()
            //
            // undoes the effects of ConvertEfiFileProtocolToShellHandle()
            //
          ConvertShellHandleToEfiFileProtocol()
            //
            // note that this does not adjust the pointer value; it's a pure
            // type cast
            //
          FileHandleClose()
            FileInterfaceMemClose()
              //
              // tears down EFI_FILE_PROTOCOL_MEM completely, undoing the
              // effects of CreateFileInterfaceMem ()
              //
    
    The FreePool() call added by bd3fc8133b2b conflicts with
    
      SHELL_FREE_NON_NULL(This);
    
    in FileInterfaceMemClose(), so remove it.
    
    This error can be reproduced for example with:
    
    > Shell> map | more
    > 'more' is not recognized as an internal or external command, operable
    > program, or script file.
    
    which triggers:
    
    > ASSERT MdeModulePkg/Core/Dxe/Mem/Pool.c(624): CR has Bad Signature
    
    with the following stack dump:
    
    > #0  0x000000007f6dc094 in CpuDeadLoop () at
    >     MdePkg/Library/BaseLib/CpuDeadLoop.c:37
    > #1  0x000000007f6dd1b4 in DebugAssert (FileName=0x7f6ed9f0
    >     "MdeModulePkg/Core/Dxe/Mem/Pool.c", LineNumber=624,
    >     Description=0x7f6ed9d8 "CR has Bad Signature") at
    >     OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c:153
    > #2  0x000000007f6d075d in CoreFreePoolI (Buffer=0x7e232c98,
    >     PoolType=0x7f6bc1c4) at MdeModulePkg/Core/Dxe/Mem/Pool.c:624
    > #3  0x000000007f6d060e in CoreInternalFreePool (Buffer=0x7e232c98,
    >     PoolType=0x7f6bc1c4) at MdeModulePkg/Core/Dxe/Mem/Pool.c:529
    > #4  0x000000007f6d0648 in CoreFreePool (Buffer=0x7e232c98) at
    >     MdeModulePkg/Core/Dxe/Mem/Pool.c:552
    > #5  0x000000007d49fbf8 in FreePool (Buffer=0x7e232c98) at
    >     MdePkg/Library/UefiMemoryAllocationLib/MemoryAllocationLib.c:818
    > #6  0x000000007d4875c3 in RunSplitCommand (CmdLine=0x7d898398,
    >     StdIn=0x0, StdOut=0x0) at ShellPkg/Application/Shell/Shell.c:1813
    > #7  0x000000007d487d59 in ProcessNewSplitCommandLine
    >     (CmdLine=0x7d898398) at ShellPkg/Application/Shell/Shell.c:2121
    > #8  0x000000007d488937 in RunShellCommand (CmdLine=0x7e233018,
    >     CommandStatus=0x0) at ShellPkg/Application/Shell/Shell.c:2670
    > #9  0x000000007d488b0b in RunCommand (CmdLine=0x7e233018) at
    >     ShellPkg/Application/Shell/Shell.c:2732
    > #10 0x000000007d4867c8 in DoShellPrompt () at
    >     ShellPkg/Application/Shell/Shell.c:1349
    > #11 0x000000007d48524d in UefiMain (ImageHandle=0x7e24c898,
    >     SystemTable=0x7f5b6018) at ShellPkg/Application/Shell/Shell.c:631
    
    Cc: Jaben Carsey <jaben.carsey@xxxxxxxxx>
    Cc: Marvin Häuser <Marvin.Haeuser@xxxxxxxxxxx>
    Cc: Qiu Shumin <shumin.qiu@xxxxxxxxx>
    Cc: Ruiyu Ni <ruiyu.ni@xxxxxxxxx>
    Fixes: bd3fc8133b2b17ad2e0427d1bf6b44b08cf2f3b2
    Contributed-under: TianoCore Contribution Agreement 1.0
    Signed-off-by: Laszlo Ersek <lersek@xxxxxxxxxx>
    Reviewed-by: Jaben Carsey <jaben.carsey@xxxxxxxxx>
    Reviewed-by: Marvin Häuser <Marvin.Haeuser@xxxxxxxxxxx>

commit 1bd0bf153ebf4993421afd5084c52a6e57e17fdc
Author: Laszlo Ersek <lersek@xxxxxxxxxx>
Date:   Tue Apr 25 14:03:32 2017 +0200

    ShellPkg/Shell: clean up bogus member types in SPLIT_LIST
    
    The "SPLIT_LIST.SplitStdOut" and "SPLIT_LIST.SplitStdIn" members currently
    have type (SHELL_FILE_HANDLE *). This is wrong; SHELL_FILE_HANDLE is
    already a pointer, there's no need to store a pointer to a pointer.
    
    The error is obvious if we check where and how these members are used:
    
    - In the RunSplitCommand() function, these members are used (populated)
      extensively; this function has to be updated in sync.
    
      ConvertEfiFileProtocolToShellHandle() already returns the temporary
      memory file created with CreateFileInterfaceMem() as SHELL_FILE_HANDLE,
      not as (SHELL_FILE_HANDLE *).
    
    - In particular, the ConvertShellHandleToEfiFileProtocol() calls need to
      be dropped as well in RunSplitCommand(), since
      EFI_SHELL_PROTOCOL.SetFilePosition() and EFI_SHELL_PROTOCOL.CloseFile()
      take SHELL_FILE_HANDLE parameters, not (EFI_FILE_PROTOCOL *).
    
      Given that ConvertShellHandleToEfiFileProtocol() only performs a
      type-cast (it does not adjust any pointer values), *and*
      SHELL_FILE_HANDLE -- taken by EFI_SHELL_PROTOCOL member functions -- is
      actually a typedef to (VOID *) -- see more on this later --, this
      conversion error hasn't been caught by compilers.
    
    - In the ProcessNewSplitCommandLine() function, RunSplitCommand() is
      called either initially (passing in NULL / NULL; no update needed), or
      recursively (passing in Split->SplitStdIn / Split->SplitStdOut; again no
      update is necessary beyond the RunSplitCommand() modification above).
    
    - In the UpdateStdInStdOutStdErr() and RestoreStdInStdOutStdErr()
      functions, said structure members are compared and assigned to
      "EFI_SHELL_PARAMETERS_PROTOCOL.StdIn" and
      "EFI_SHELL_PARAMETERS_PROTOCOL.StdOut", both of which have type
      SHELL_FILE_HANDLE, *not* (SHELL_FILE_HANDLE *).
    
      The compiler hasn't caught this error because of the fatally flawed type
      definition of SHELL_FILE_HANDLE, namely
    
        typedef VOID *SHELL_FILE_HANDLE;
    
      Pointer-to-void silently converts to and from most other pointer types;
      among them, pointer-to-pointer-to-void. That is also why no update is
      necessary for UpdateStdInStdOutStdErr() and RestoreStdInStdOutStdErr()
      in this fix.
    
    (
    
    Generally speaking, using (VOID *) typedefs for opaque handles is a tragic
    mistake in all of the UEFI-related specifications; this practice defeats
    any type checking that compilers might help programmers with. The right
    way to define an opaque handle is as follows:
    
      //
      // Introduce the incomplete structure type, and the derived pointer
      // type, in both the specification and the public edk2 headers. Note
      // that the derived pointer type itself is a complete type, and it can
      // be used freely by client code.
      //
      typedef struct SHELL_FILE *SHELL_FILE_HANDLE;
    
      //
      // Complete the structure type in the edk2 internal C source files.
      //
      struct SHELL_FILE {
        //
        // list fields
        //
      };
    
    This way the structure size and members remain hidden from client code,
    but the C compiler can nonetheless catch any invalid conversions between
    incompatible XXX_HANDLE types.
    
    )
    
    Cc: Jaben Carsey <jaben.carsey@xxxxxxxxx>
    Cc: Marvin Häuser <Marvin.Haeuser@xxxxxxxxxxx>
    Cc: Qiu Shumin <shumin.qiu@xxxxxxxxx>
    Cc: Ruiyu Ni <ruiyu.ni@xxxxxxxxx>
    Contributed-under: TianoCore Contribution Agreement 1.0
    Signed-off-by: Laszlo Ersek <lersek@xxxxxxxxxx>
    Reviewed-by: Jaben Carsey <jaben.carsey@xxxxxxxxx>

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.