Re: [PATCH iproute2-rc 1/2] rdma: Fix core dump when pretty is used

[Chengchang Tang]

On 2023/12/30 1:21, Stephen Hemminger wrote:
> On Fri, 29 Dec 2023 14:52:40 +0800
> Junxian Huang <huangjunxian6@xxxxxxxxxxxxx> wrote:
>
> > From: Chengchang Tang <tangchengchang@xxxxxxxxxx>
> >
> > There will be a core dump when pretty is used as the JSON object
> > hasn't been opened and closed properly.
> >
> > Before:
> > $ rdma res show qp -jp -dd
> > [ {
> >      "ifindex": 1,
> >      "ifname": "hns_1",
> >      "port": 1,
> >      "lqpn": 1,
> >      "type": "GSI",
> >      "state": "RTS",
> >      "sq-psn": 0,
> >      "comm": "ib_core"
> > },
> > "drv_sq_wqe_cnt": 128,
> > "drv_sq_max_gs": 2,
> > "drv_rq_wqe_cnt": 512,
> > "drv_rq_max_gs": 1,
> > rdma: json_writer.c:130: jsonw_end: Assertion `self->depth > 0' failed.
> > Aborted (core dumped)
> >
> > After:
> > $ rdma res show qp -jp -dd
> > [ {
> >          "ifindex": 2,
> >          "ifname": "hns_2",
> >          "port": 1,
> >          "lqpn": 1,
> >          "type": "GSI",
> >          "state": "RTS",
> >          "sq-psn": 0,
> >          "comm": "ib_core",{
> >              "drv_sq_wqe_cnt": 128,
> >              "drv_sq_max_gs": 2,
> >              "drv_rq_wqe_cnt": 512,
> >              "drv_rq_max_gs": 1,
> >              "drv_ext_sge_sge_cnt": 256
> >          }
> >      } ]
> >
> > Fixes: 331152752a97 ("rdma: print driver resource attributes")
> > Signed-off-by: Chengchang Tang <tangchengchang@xxxxxxxxxx>
> > Signed-off-by: Junxian Huang <huangjunxian6@xxxxxxxxxxxxx>
> This code in rdma seems to be miking json and newline functionality
> which creates bug traps.
>
> Also the json should have same effective output in pretty and non-pretty mode.
> It looks like since pretty mode add extra object layer, the nesting of {} would be
> different.
>
> The conversion to json_print() was done but it isn't using same conventions
> as ip or tc.
>
> The correct fix needs to go deeper and hit other things.

Hi, Stephen,

The root cause of this issue is that close_json_object() is being called 
in newline_indent(), resulting in a mismatch
of {}.

When fixing this problem, I was unsure why a newline() is needed in 
pretty mode, so I simply kept this logic and
solved the issue of open_json_object() and close_json_object() not 
matching. However, If the output of pretty mode
and not-pretty mode should be the same, then this problem can be 
resolved by deleting this newline_indent().

I believe the original developer may not have realized that 
close_json_object() is being called in newline(), which leads
to this problem. To improve the code's readability, I would try to strip 
out close_json_obejct() from newline().

Thanks,
Chengchang Tang