<div dir="ltr"><div>On Sat, Jun 17, 2017 at 12:22:48AM +0300, Dmitry V. Levin wrote:</div><div>> On Fri, Jun 16, 2017 at 01:35:07PM +0800, JingPiao Chen wrote:</div><div>> > * defs.h (decode_nlattr): New prototype.</div><div>> > * linux/unix_diag.h (UNIX_DIAG_*): New enum.</div><div>> > * netlink.c (fetch_nlattr, print_nlattr,</div><div>> > decode_nlattr_with_data, decode_nlattr): New functions.</div><div>> </div><div>> Looks like there is no necessity to place these new nlattr related</div><div>> functions into netlink.c, what do you think about creating a new source</div><div>> file for this purpose, e.g. nlattr.c?</div><div>> </div><div><br></div><div>I like create new file nalttr.c.</div><div><br></div><div>> > * netlink_sock_diag.c: Include "xlat/unix_diag_attrs.h".</div><div>> > (decode_unix_diag_msg): Use decode_nlattr.</div><div>> > * xlat/<a href="http://unix_diag_attrs.in">unix_diag_attrs.in</a>: New file.</div><div>> > </div><div>> > co-authored-by: Fabien Siron <<a href="mailto:fabien.siron@epita.fr">fabien.siron@epita.fr</a>></div><div>> > ---</div><div>> >  defs.h                  |  3 ++</div><div>> >  linux/unix_diag.h       | 11 +++++--</div><div>> >  netlink.c               | 76 +++++++++++++++++++++++++++++++++++++++++++++++++</div><div>> >  netlink_sock_diag.c     |  4 +++</div><div>> >  xlat/<a href="http://unix_diag_attrs.in">unix_diag_attrs.in</a> |  7 +++++</div><div>> >  5 files changed, 99 insertions(+), 2 deletions(-)</div><div>> >  create mode 100644 xlat/<a href="http://unix_diag_attrs.in">unix_diag_attrs.in</a></div><div>> > </div><div>> > diff --git a/defs.h b/defs.h</div><div>> > index 487e51b..7e3912d 100644</div><div>> > --- a/defs.h</div><div>> > +++ b/defs.h</div><div>> > @@ -632,6 +632,9 @@ tprint_iov_upto(struct tcb *, kernel_ulong_t len, kernel_ulong_t addr,</div><div>> >  </div><div>> >  extern void</div><div>> >  decode_netlink(struct tcb *, int fd, kernel_ulong_t addr, kernel_ulong_t len);</div><div>> > +extern void</div><div>> > +decode_nlattr(struct tcb *tcp, kernel_ulong_t addr, kernel_ulong_t len,</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">     </span>      int, const struct xlat *, const char *);</div><div>> >  </div><div>> >  extern void tprint_open_modes(unsigned int);</div><div>> >  extern const char *sprint_open_modes(unsigned int);</div><div>> > diff --git a/linux/unix_diag.h b/linux/unix_diag.h</div><div>> > index a6b62ba..3c9d99f 100644</div><div>> > --- a/linux/unix_diag.h</div><div>> > +++ b/linux/unix_diag.h</div><div>> > @@ -27,7 +27,14 @@ struct unix_diag_msg {</div><div>> >  <span class="gmail-Apple-tab-span" style="white-space:pre">       </span>uint32_t udiag_cookie[2];</div><div>> >  };</div><div>> >  </div><div>> > -#define UNIX_DIAG_NAME 0</div><div>> > -#define UNIX_DIAG_PEER 2</div><div>> > +enum {</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">       </span>UNIX_DIAG_NAME,</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">     </span>UNIX_DIAG_VFS,</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">      </span>UNIX_DIAG_PEER,</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">     </span>UNIX_DIAG_ICONS,</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">    </span>UNIX_DIAG_RQLEN,</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">    </span>UNIX_DIAG_MEMINFO,</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">  </span>UNIX_DIAG_SHUTDOWN,</div><div>> > +};</div><div>> >  </div><div>> >  #endif /* !STRACE_LINUX_UNIX_DIAG_H */</div><div>> > diff --git a/netlink.c b/netlink.c</div><div>> > index 4bef949..52f3806 100644</div><div>> > --- a/netlink.c</div><div>> > +++ b/netlink.c</div><div>> > @@ -49,6 +49,82 @@</div><div>> >  #undef NLMSG_HDRLEN</div><div>> >  #define NLMSG_HDRLEN NLMSG_ALIGN(sizeof(struct nlmsghdr))</div><div>> >  </div><div>> > +static bool</div><div>> > +fetch_nlattr(struct tcb *tcp, struct nlattr *nlattr,</div><div>> </div><div>> You can add some const qualifiers here, like in fetch_nlmsghdr that was</div><div>> used as a template for this new function.</div><div>> </div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">   </span>     const kernel_ulong_t addr, const kernel_ulong_t len)</div><div>> > +{</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">      </span>if (len < sizeof(struct nlattr)) {</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">               </span>printstrn(tcp, addr, len);</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">          </span>return false;</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">       </span>}</div><div>> > +</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre"> </span>if (umove_or_printaddr(tcp, addr, nlattr))</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">          </span>return false;</div><div>> > +</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">     </span>return true;</div><div>> > +}</div><div>> > +</div><div>> > +static void</div><div>> > +print_nlattr(const struct nlattr *const nla,</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">        </span>     const struct xlat *table, const char *dflt)</div><div>> > +{</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">       </span>tprintf("{nla_len=%u, nla_type=", nla->nla_len);</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">       </span>if (nla->nla_type & NLA_F_NESTED)</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">            </span>tprints("NLA_F_NESTED|");</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre"> </span>if (nla->nla_type & NLA_F_NET_BYTEORDER)</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">             </span>tprints("NLA_F_NET_BYTEORDER|");</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">  </span>printxval(table, nla->nla_type & NLA_TYPE_MASK, dflt);</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">       </span>tprints("}");</div><div>> > +}</div><div>> > +</div><div>> > +static void</div><div>> > +decode_nlattr_with_data(struct tcb *tcp, kernel_ulong_t addr,</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">                    </span>kernel_ulong_t len, const struct xlat *table,</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">                       </span>const char *dflt)</div><div>> > +{</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">        </span>struct nlattr nlattr;</div><div>> > +</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">     </span>while (fetch_nlattr(tcp, &nlattr, addr, len)) {</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">         </span>unsigned long nlattr_len = NLA_ALIGN(nlattr.nla_len);</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">               </span>kernel_ulong_t next_addr = 0;</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">               </span>kernel_ulong_t next_len = 0;</div><div>> </div><div>> This new function is made after decode_netlink, which is fine,</div><div>> but it misses the abbrev(tcp) logic which is present in decode_netlink.</div><div>> Could you reintroduce it here, too, please?</div><div>> </div><div>> > +</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">                </span>if (nlattr.nla_len >= sizeof(struct nlattr)) {</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">                   </span>next_len = (len >= nlattr_len) ? len - nlattr_len : 0;</div><div>> > +</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">                 </span>if (next_len && addr + nlattr_len > addr)</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">                                </span>next_addr = addr + nlattr_len;</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">              </span>}</div><div>> > +</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">         </span>tprints("{");</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">             </span>print_nlattr(&nlattr, table, dflt);</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">             </span>if (nlattr.nla_len > NLA_HDRLEN) {</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">                       </span>tprints(", ");</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">                    </span>printstrn(tcp, addr + NLA_HDRLEN,</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">                           </span>  nlattr.nla_len - NLA_HDRLEN);</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">            </span>}</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">           </span>tprints("}");</div><div>> > +</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">           </span>if (!next_addr)</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">                     </span>break;</div><div>> > +</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">            </span>tprints(", ");</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">            </span>addr = next_addr;</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">           </span>len = next_len;</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">     </span>}</div><div>> > +}</div><div>> > +</div><div>> > +void</div><div>> > +decode_nlattr(struct tcb *tcp, kernel_ulong_t addr, kernel_ulong_t len,</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">       </span>      int hdrlen, const struct xlat *table, const char *dflt)</div><div>> > +{</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre"> </span>if (len > NLMSG_ALIGN(hdrlen)) {</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">         </span>tprints(", ");</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">            </span>decode_nlattr_with_data(tcp, addr + NLMSG_ALIGN(hdrlen),</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">                                    </span>len - NLMSG_ALIGN(hdrlen), table, dflt);</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">    </span>}</div><div>> </div><div>> I'm not sure we need this proxy function, the caller is capable of performing</div><div>> the check and invoking the decoder with the right address and length.</div><div><br></div><div>Yes, but delete this function will add a lot of code,</div><div>and the check is scattered.</div><div><br></div><div>> > +}</div><div>> > +</div><div>> >  /*</div><div>> >   * Fetch a struct nlmsghdr from the given address.</div><div>> >   */</div><div>> > diff --git a/netlink_sock_diag.c b/netlink_sock_diag.c</div><div>> > index 8dbfd07..6d79ba4 100644</div><div>> > --- a/netlink_sock_diag.c</div><div>> > +++ b/netlink_sock_diag.c</div><div>> > @@ -55,6 +55,7 @@</div><div>> >  # include "xlat/smc_states.h"</div><div>> >  #endif</div><div>> >  </div><div>> > +#include "xlat/unix_diag_attrs.h"</div><div>> >  #include "xlat/unix_diag_show.h"</div><div>> >  </div><div>> >  static void</div><div>> > @@ -109,32 +110,35 @@ static void</div><div>> >  decode_unix_diag_msg(struct tcb *const tcp,</div><div>> >  <span class="gmail-Apple-tab-span" style="white-space:pre">              </span>     const struct nlmsghdr *const nlmsghdr,</div><div>> >  <span class="gmail-Apple-tab-span" style="white-space:pre">              </span>     const uint8_t family,</div><div>> >  <span class="gmail-Apple-tab-span" style="white-space:pre">               </span>     const kernel_ulong_t addr,</div><div>> >  <span class="gmail-Apple-tab-span" style="white-space:pre">          </span>     const kernel_ulong_t len)</div><div>> >  {</div><div>> >  <span class="gmail-Apple-tab-span" style="white-space:pre">       </span>struct unix_diag_msg msg = { .udiag_family = family };</div><div>> >  <span class="gmail-Apple-tab-span" style="white-space:pre">     </span>const size_t offset = sizeof(msg.udiag_family);</div><div>> >  </div><div>> >  <span class="gmail-Apple-tab-span" style="white-space:pre"> </span>tprints("{udiag_family=");</div><div>> >  <span class="gmail-Apple-tab-span" style="white-space:pre">       </span>printxval(addrfams, msg.udiag_family, "AF_???");</div><div>> >  </div><div>> >  <span class="gmail-Apple-tab-span" style="white-space:pre">      </span>tprints(", ");</div><div>> >  <span class="gmail-Apple-tab-span" style="white-space:pre">   </span>if (len >= sizeof(msg)) {</div><div>> >  <span class="gmail-Apple-tab-span" style="white-space:pre">               </span>if (!umoven_or_printaddr(tcp, addr + offset,</div><div>> >  <span class="gmail-Apple-tab-span" style="white-space:pre">                                       </span> sizeof(msg) - offset,</div><div>> >  <span class="gmail-Apple-tab-span" style="white-space:pre">                                     </span> (void *) &msg + offset)) {</div><div>> >  <span class="gmail-Apple-tab-span" style="white-space:pre">                    </span>tprints("udiag_type=");</div><div>> >  <span class="gmail-Apple-tab-span" style="white-space:pre">                  </span>printxval(socktypes, msg.udiag_type, "SOCK_???");</div><div>> >  <span class="gmail-Apple-tab-span" style="white-space:pre">                        </span>tprintf(", udiag_state=");</div><div>> >  <span class="gmail-Apple-tab-span" style="white-space:pre">                       </span>printxval(tcp_states, msg.udiag_state, "TCP_???");</div><div>> >  <span class="gmail-Apple-tab-span" style="white-space:pre">                       </span>tprintf(", udiag_ino=%" PRIu32</div><div>> >  <span class="gmail-Apple-tab-span" style="white-space:pre">                           </span>", udiag_cookie=[%" PRIu32 ", %" PRIu32 "]",</div><div>> >  <span class="gmail-Apple-tab-span" style="white-space:pre">                         </span>msg.udiag_ino,</div><div>> >  <span class="gmail-Apple-tab-span" style="white-space:pre">                             </span>msg.udiag_cookie[0], msg.udiag_cookie[1]);</div><div>> </div><div>> The invocation of nlattr decoder should be placed here:</div><div>> </div><div>> <span class="gmail-Apple-tab-span" style="white-space:pre">                   </span>offset = NLMSG_ALIGN(sizeof(msg));</div><div>> <span class="gmail-Apple-tab-span" style="white-space:pre">                        </span>if (len > offset) {</div><div>> <span class="gmail-Apple-tab-span" style="white-space:pre">                            </span>tprints(", ");</div><div>> <span class="gmail-Apple-tab-span" style="white-space:pre">                          </span>decode_nlattr(tcp, addr + offset, len - offset,</div><div>> <span class="gmail-Apple-tab-span" style="white-space:pre">                                   </span>      unix_diag_attrs, "UNIX_DIAG_???");</div><div>> <span class="gmail-Apple-tab-span" style="white-space:pre">                     </span>}</div><div>> >  <span class="gmail-Apple-tab-span" style="white-space:pre">          </span>}</div><div>> >  <span class="gmail-Apple-tab-span" style="white-space:pre">  </span>} else</div><div>> >  <span class="gmail-Apple-tab-span" style="white-space:pre">             </span>tprints("...");</div><div>> >  <span class="gmail-Apple-tab-span" style="white-space:pre">  </span>tprints("}");</div><div>> > +</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">   </span>decode_nlattr(tcp, addr, len, sizeof(msg), unix_diag_attrs,</div><div>> > +<span class="gmail-Apple-tab-span" style="white-space:pre">         </span>      "UNIX_DIAG_???");</div><div>> >  }</div><div>> >  </div><div>> >  static void</div><div>> > diff --git a/xlat/<a href="http://unix_diag_attrs.in">unix_diag_attrs.in</a> b/xlat/<a href="http://unix_diag_attrs.in">unix_diag_attrs.in</a></div><div>> > new file mode 100644</div><div>> > index 0000000..0fb2f23</div><div>> > --- /dev/null</div><div>> > +++ b/xlat/<a href="http://unix_diag_attrs.in">unix_diag_attrs.in</a></div><div>> > @@ -0,0 +1,7 @@</div><div>> > +UNIX_DIAG_NAME<span class="gmail-Apple-tab-span" style="white-space:pre">            </span>0</div><div>> > +UNIX_DIAG_VFS<span class="gmail-Apple-tab-span" style="white-space:pre">              </span>1</div><div>> > +UNIX_DIAG_PEER<span class="gmail-Apple-tab-span" style="white-space:pre">             </span>2</div><div>> > +UNIX_DIAG_ICONS<span class="gmail-Apple-tab-span" style="white-space:pre">            </span>3</div><div>> > +UNIX_DIAG_RQLEN<span class="gmail-Apple-tab-span" style="white-space:pre">            </span>4</div><div>> > +UNIX_DIAG_MEMINFO<span class="gmail-Apple-tab-span" style="white-space:pre">  </span>5</div><div>> > +UNIX_DIAG_SHUTDOWN<span class="gmail-Apple-tab-span" style="white-space:pre"> </span>6</div><div>> </div><div>> As these constants are guaranteed to be defined by local linux/unix_diag.h</div><div>> file, there is no need to provide fallback definitions, just the opposite,</div><div>> these xlat entries should be unconditional:</div><div>> </div><div>> --- /dev/null</div><div>> +++ b/xlat/<a href="http://unix_diag_attrs.in">unix_diag_attrs.in</a></div><div>> @@ -0,0 +1,8 @@</div><div>> +#unconditional</div><div>> +UNIX_DIAG_NAME</div><div>> +UNIX_DIAG_VFS</div><div>> +UNIX_DIAG_PEER</div><div>> +UNIX_DIAG_ICONS</div><div>> +UNIX_DIAG_RQLEN</div><div>> +UNIX_DIAG_MEMINFO</div><div>> +UNIX_DIAG_SHUTDOWN</div><div><br></div><div>--</div><div>JingPiao Chen</div><div><br></div></div>