qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: "Alex Bennée" <alex.bennee@linaro.org>, "Li Qiang" <liq3ea@gmail.com>
Cc: "Philippe Mathieu-Daudé" <philmd@redhat.com>,
	"Jason Wang" <jasowang@redhat.com>, "Li Qiang" <liq3ea@163.com>,
	qemu-devel@nongnu.org, "Michael S. Tsirkin" <mst@redhat.com>
Subject: Re: [PATCH] virtio: vdpa: omit check return of g_malloc
Date: Wed, 23 Sep 2020 13:06:15 -0500	[thread overview]
Message-ID: <a5d9d78c-fd3e-38a9-62b6-d0d7b1081549@redhat.com> (raw)
In-Reply-To: <87r1qzw7nz.fsf@linaro.org>

On 9/18/20 8:12 AM, Alex Bennée wrote:
> 
> Li Qiang <liq3ea@gmail.com> writes:
> 
>> Philippe Mathieu-Daudé <philmd@redhat.com> 于2020年8月19日周三 下午11:07写道:
>>>
>>> On 8/19/20 4:43 PM, Li Qiang wrote:
>>>> If g_malloc fails, the application will be terminated.
>>>
>>> Which we don't want... better to use g_try_malloc() instead?
>>
>> I don't think so. If g_malloc return NULL it means a critical
>> situation I think terminate the application
>> is OK. Though I don't find any rule/practices the qemu code base uses
>> g_malloc far more than
>> g_try_malloc.
> 
> g_try_malloc is only for cases you could recover from, by either
> deferring or doing something else. A straight out of memory failure is
> fatal.
> 
> Arguably a bunch of the try_malloc's in the code base should be straight
> mallocs. The ELF loaders load_symbols does it because I guess having the
> symbols is a bonus and you could still run the program if a) there was
> enough memory to run and b) your symbol table was very large.

If the amount of memory being allocated is under user control (such as 
from an elf header or qcow2 image), you want g_try_malloc() to prevent 
against malicious users requesting outrageous amounts.  But if it is 
solely under programmer control, where the maximum amount requested is 
not going to be a problem unless you are already truly out of memory for 
other reasons, g_malloc() is appropriate.  A potential rule of thumb 
might be that if you know it is less than 1M of memory, it's not worth 
trying to recover from.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org



  reply	other threads:[~2020-09-23 18:07 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-19 14:43 [PATCH] virtio: vdpa: omit check return of g_malloc Li Qiang
2020-08-19 15:07 ` Philippe Mathieu-Daudé
2020-08-20  1:26   ` Li Qiang
2020-09-18 13:12     ` Alex Bennée
2020-09-23 18:06       ` Eric Blake [this message]
2020-09-18 12:53 ` Laurent Vivier
2020-09-23 17:08   ` Laurent Vivier
2020-09-18 13:14 ` Alex Bennée

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=a5d9d78c-fd3e-38a9-62b6-d0d7b1081549@redhat.com \
    --to=eblake@redhat.com \
    --cc=alex.bennee@linaro.org \
    --cc=jasowang@redhat.com \
    --cc=liq3ea@163.com \
    --cc=liq3ea@gmail.com \
    --cc=mst@redhat.com \
    --cc=philmd@redhat.com \
    --cc=qemu-devel@nongnu.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).